On Wed, Feb 12, 2025 at 11:21:25AM +0100, Pavel Hrdina wrote:
The commit in question made an incorrect change that resulted in getting O_RDONLY FD instead of O_RDWR preventing any writes to happen with the following error:
virQEMUSaveDataWrite:176 : failed to write header to domain save file '/path/to/save.img': Bad file descriptor
Fixes: 517248e2394476a3105ff5866b0b718fc6583073 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78bfaa5b3a..a35abf2747 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5949,7 +5949,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, if (qemuSaveImageGetMetadata(driver, NULL, path, &def, &data) < 0) goto cleanup;
- fd = qemuSaveImageOpen(driver, path, 0, NULL, false); + fd = qemuSaveImageOpen(driver, path, false, NULL, true); if (fd < 0) goto cleanup;
Not an objection to your patch, just an observation about API design in general. Parameters which are 'bool' flags actively encourage this kind of mistake, even more so when there are multiple bools per API. This is why of the reasons that glib has a general design practice of defining enums for any flags. eg typedef enum { QEMU_SAVE_IMAGE_OPEN_BYPASS_CACHE = (1 << 0), QEMU_SAVE_IMAGE_OPEN_WRITABLE = (1 << 1), } qemuSaveImageFlags; qemuSaveImageOpen(virQEMUDriver *driver, const char *path, enumqemuSaveImageFlags falgs, virFileWrapperFd **wrapperFd); With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|