[libvirt] [PATCH 0/2] qemu: Don't format 'cache' for empty cdroms
See patch 2/2 for explanation. Peter Krempa (2): tests: qemuxml2argv: Add test case for empty CDROM with cache mode qemu: command: Don't format image properties for empty -drive src/qemu/qemu_command.c | 47 ++++++++++--------- tests/qemuxml2argvdata/disk-cdrom.args | 6 ++- .../disk-cdrom.x86_64-2.12.0.args | 9 ++-- .../disk-cdrom.x86_64-latest.args | 9 ++-- tests/qemuxml2argvdata/disk-cdrom.xml | 6 +++ tests/qemuxml2xmloutdata/disk-cdrom.xml | 6 +++ 6 files changed, 52 insertions(+), 31 deletions(-) -- 2.20.1
Upcomming change will influence CDROM with cache mode so add a test case. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/disk-cdrom.args | 4 +++- tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args | 7 +++++-- tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args | 7 +++++-- tests/qemuxml2argvdata/disk-cdrom.xml | 6 ++++++ tests/qemuxml2xmloutdata/disk-cdrom.xml | 6 ++++++ 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/tests/qemuxml2argvdata/disk-cdrom.args b/tests/qemuxml2argvdata/disk-cdrom.args index 6f78a314a9..0b62e6ee9f 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.args +++ b/tests/qemuxml2argvdata/disk-cdrom.args @@ -24,8 +24,10 @@ server,nowait \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ bootindex=1 \ --drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-1-0,media=cdrom,\ +-drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,media=cdrom,\ readonly=on \ +-device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ +-drive if=none,id=drive-ide0-1-0,media=cdrom,readonly=on,cache=none \ -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ -drive if=none,id=drive-ide0-1-1,media=cdrom,readonly=on \ -device ide-drive,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args index 59607258eb..b51c0919cc 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args +++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args @@ -26,8 +26,11 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ --drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-1-0,readonly=on \ --device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ +-drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \ +-device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ +-drive if=none,id=drive-ide0-1-0,readonly=on,cache=none \ +-device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\ +write-cache=on \ -drive if=none,id=drive-ide0-1-1,readonly=on \ -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args index 4c5a599820..8bdcffada3 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args @@ -26,8 +26,11 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ --drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-1-0,readonly=on \ --device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ +-drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \ +-device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ +-drive if=none,id=drive-ide0-1-0,readonly=on,cache=none \ +-device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\ +write-cache=on \ -drive if=none,id=drive-ide0-1-1,readonly=on \ -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ diff --git a/tests/qemuxml2argvdata/disk-cdrom.xml b/tests/qemuxml2argvdata/disk-cdrom.xml index be229657f7..f5e9f7395d 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.xml +++ b/tests/qemuxml2argvdata/disk-cdrom.xml @@ -23,6 +23,12 @@ <disk type='file' device='cdrom'> <driver name='qemu' type='raw'/> <source file='/root/boot.iso'/> + <target dev='hdb' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw' cache='none'/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> diff --git a/tests/qemuxml2xmloutdata/disk-cdrom.xml b/tests/qemuxml2xmloutdata/disk-cdrom.xml index e96c94097a..7696785930 100644 --- a/tests/qemuxml2xmloutdata/disk-cdrom.xml +++ b/tests/qemuxml2xmloutdata/disk-cdrom.xml @@ -23,6 +23,12 @@ <disk type='file' device='cdrom'> <driver name='qemu' type='raw'/> <source file='/root/boot.iso'/> + <target dev='hdb' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw' cache='none'/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> -- 2.20.1
On Wed, Jan 16, 2019 at 10:42:09AM +0100, Peter Krempa wrote:
Upcomming change will influence CDROM with cache mode so add a test case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/disk-cdrom.args | 4 +++- tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args | 7 +++++-- tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args | 7 +++++-- tests/qemuxml2argvdata/disk-cdrom.xml | 6 ++++++ tests/qemuxml2xmloutdata/disk-cdrom.xml | 6 ++++++ 5 files changed, 25 insertions(+), 5 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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 :|
If a -drive has no image, using image properties makes qemu whine that they should not be used. This patch stops formating cache/readonly/... for empty drives for the pre-blockdev syntax. Unfortunately those parameters can't be added later when inserting media, but on the other hand qemu will start with an empty drive. Since we already were able to start a VM with such config previously due to qemu ignoring them I've opted just to skip formatting them. Additionally with -blockdev support it will work as expected as the image properties will be formatted when adding the image itself which is not possible without it. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1651457 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 47 ++++++++++--------- tests/qemuxml2argvdata/disk-cdrom.args | 4 +- .../disk-cdrom.x86_64-2.12.0.args | 4 +- .../disk-cdrom.x86_64-latest.args | 4 +- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 822d5f8669..3913ac4c15 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1745,37 +1745,38 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } } - if (disk->src->readonly) - virBufferAddLit(&opt, ",readonly=on"); + if (!virStorageSourceIsEmpty(disk->src)) { + if (disk->src->readonly) + virBufferAddLit(&opt, ",readonly=on"); + if (disk->cachemode) { + virBufferAsprintf(&opt, ",cache=%s", + qemuDiskCacheV2TypeToString(disk->cachemode)); + } - if (disk->cachemode) { - virBufferAsprintf(&opt, ",cache=%s", - qemuDiskCacheV2TypeToString(disk->cachemode)); - } + if (disk->copy_on_read) { + virBufferAsprintf(&opt, ",copy-on-read=%s", + virTristateSwitchTypeToString(disk->copy_on_read)); + } - if (disk->copy_on_read) { - virBufferAsprintf(&opt, ",copy-on-read=%s", - virTristateSwitchTypeToString(disk->copy_on_read)); - } + if (disk->discard) { + virBufferAsprintf(&opt, ",discard=%s", + virDomainDiskDiscardTypeToString(disk->discard)); + } - if (disk->discard) { - virBufferAsprintf(&opt, ",discard=%s", - virDomainDiskDiscardTypeToString(disk->discard)); - } + if (detect_zeroes) { + virBufferAsprintf(&opt, ",detect-zeroes=%s", + virDomainDiskDetectZeroesTypeToString(detect_zeroes)); + } - if (detect_zeroes) { - virBufferAsprintf(&opt, ",detect-zeroes=%s", - virDomainDiskDetectZeroesTypeToString(detect_zeroes)); - } + if (disk->iomode) { + virBufferAsprintf(&opt, ",aio=%s", + virDomainDiskIoTypeToString(disk->iomode)); + } - if (disk->iomode) { - virBufferAsprintf(&opt, ",aio=%s", - virDomainDiskIoTypeToString(disk->iomode)); + qemuBuildDiskThrottling(disk, &opt); } - qemuBuildDiskThrottling(disk, &opt); - if (virBufferCheckError(&opt) < 0) goto error; diff --git a/tests/qemuxml2argvdata/disk-cdrom.args b/tests/qemuxml2argvdata/disk-cdrom.args index 0b62e6ee9f..a9f60aa477 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.args +++ b/tests/qemuxml2argvdata/disk-cdrom.args @@ -27,7 +27,7 @@ bootindex=1 \ -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,media=cdrom,\ readonly=on \ -device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive if=none,id=drive-ide0-1-0,media=cdrom,readonly=on,cache=none \ +-drive if=none,id=drive-ide0-1-0,media=cdrom \ -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ --drive if=none,id=drive-ide0-1-1,media=cdrom,readonly=on \ +-drive if=none,id=drive-ide0-1-1,media=cdrom \ -device ide-drive,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args index b51c0919cc..a39d920f67 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args +++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args @@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \ -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive if=none,id=drive-ide0-1-0,readonly=on,cache=none \ +-drive if=none,id=drive-ide0-1-0 \ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\ write-cache=on \ --drive if=none,id=drive-ide0-1-1,readonly=on \ +-drive if=none,id=drive-ide0-1-1 \ -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args index 8bdcffada3..029ae23dfa 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args @@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \ -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive if=none,id=drive-ide0-1-0,readonly=on,cache=none \ +-drive if=none,id=drive-ide0-1-0 \ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\ write-cache=on \ --drive if=none,id=drive-ide0-1-1,readonly=on \ +-drive if=none,id=drive-ide0-1-1 \ -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ -- 2.20.1
On Wed, Jan 16, 2019 at 10:42:10AM +0100, Peter Krempa wrote:
If a -drive has no image, using image properties makes qemu whine that they should not be used.
This patch stops formating cache/readonly/... for empty drives for the pre-blockdev syntax. Unfortunately those parameters can't be added later when inserting media, but on the other hand qemu will start with an empty drive.
So, IIUC, if you need to set cache/readonly/etc when inserting media, then the fix is simply to use a QEMU that supports blockdev?
Since we already were able to start a VM with such config previously due to qemu ignoring them I've opted just to skip formatting them. Additionally with -blockdev support it will work as expected as the image properties will be formatted when adding the image itself which is not possible without it.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1651457
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 47 ++++++++++--------- tests/qemuxml2argvdata/disk-cdrom.args | 4 +- .../disk-cdrom.x86_64-2.12.0.args | 4 +- .../disk-cdrom.x86_64-latest.args | 4 +- 4 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 822d5f8669..3913ac4c15 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1745,37 +1745,38 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } }
- if (disk->src->readonly) - virBufferAddLit(&opt, ",readonly=on"); + if (!virStorageSourceIsEmpty(disk->src)) { + if (disk->src->readonly) + virBufferAddLit(&opt, ",readonly=on");
+ if (disk->cachemode) { + virBufferAsprintf(&opt, ",cache=%s", + qemuDiskCacheV2TypeToString(disk->cachemode)); + }
- if (disk->cachemode) { - virBufferAsprintf(&opt, ",cache=%s", - qemuDiskCacheV2TypeToString(disk->cachemode)); - } + if (disk->copy_on_read) { + virBufferAsprintf(&opt, ",copy-on-read=%s", + virTristateSwitchTypeToString(disk->copy_on_read)); + }
- if (disk->copy_on_read) { - virBufferAsprintf(&opt, ",copy-on-read=%s", - virTristateSwitchTypeToString(disk->copy_on_read)); - } + if (disk->discard) { + virBufferAsprintf(&opt, ",discard=%s", + virDomainDiskDiscardTypeToString(disk->discard)); + }
- if (disk->discard) { - virBufferAsprintf(&opt, ",discard=%s", - virDomainDiskDiscardTypeToString(disk->discard)); - } + if (detect_zeroes) { + virBufferAsprintf(&opt, ",detect-zeroes=%s", + virDomainDiskDetectZeroesTypeToString(detect_zeroes)); + }
- if (detect_zeroes) { - virBufferAsprintf(&opt, ",detect-zeroes=%s", - virDomainDiskDetectZeroesTypeToString(detect_zeroes)); - } + if (disk->iomode) { + virBufferAsprintf(&opt, ",aio=%s", + virDomainDiskIoTypeToString(disk->iomode)); + }
- if (disk->iomode) { - virBufferAsprintf(&opt, ",aio=%s", - virDomainDiskIoTypeToString(disk->iomode)); + qemuBuildDiskThrottling(disk, &opt); }
- qemuBuildDiskThrottling(disk, &opt); - if (virBufferCheckError(&opt) < 0) goto error;
diff --git a/tests/qemuxml2argvdata/disk-cdrom.args b/tests/qemuxml2argvdata/disk-cdrom.args index 0b62e6ee9f..a9f60aa477 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.args +++ b/tests/qemuxml2argvdata/disk-cdrom.args @@ -27,7 +27,7 @@ bootindex=1 \ -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,media=cdrom,\ readonly=on \ -device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive if=none,id=drive-ide0-1-0,media=cdrom,readonly=on,cache=none \ +-drive if=none,id=drive-ide0-1-0,media=cdrom \ -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ --drive if=none,id=drive-ide0-1-1,media=cdrom,readonly=on \ +-drive if=none,id=drive-ide0-1-1,media=cdrom \ -device ide-drive,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args index b51c0919cc..a39d920f67 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args +++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args @@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \ -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive if=none,id=drive-ide0-1-0,readonly=on,cache=none \ +-drive if=none,id=drive-ide0-1-0 \ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\ write-cache=on \ --drive if=none,id=drive-ide0-1-1,readonly=on \ +-drive if=none,id=drive-ide0-1-1 \ -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args index 8bdcffada3..029ae23dfa 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args @@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \ -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive if=none,id=drive-ide0-1-0,readonly=on,cache=none \ +-drive if=none,id=drive-ide0-1-0 \ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\ write-cache=on \ --drive if=none,id=drive-ide0-1-1,readonly=on \ +-drive if=none,id=drive-ide0-1-1 \ -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ -- 2.20.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 :|
On Wed, Jan 16, 2019 at 09:52:42 +0000, Daniel Berrange wrote:
On Wed, Jan 16, 2019 at 10:42:10AM +0100, Peter Krempa wrote:
If a -drive has no image, using image properties makes qemu whine that they should not be used.
This patch stops formating cache/readonly/... for empty drives for the pre-blockdev syntax. Unfortunately those parameters can't be added later when inserting media, but on the other hand qemu will start with an empty drive.
So, IIUC, if you need to set cache/readonly/etc when inserting media, then the fix is simply to use a QEMU that supports blockdev?
Yes. With blockdev we setup the cache modes as the user wishes/configures. Currently AFAIK the user would not get the required mode anyways. This should not be any problem for cdroms. Floppy drives might end up with incorrect cache mode.
On Wed, Jan 16, 2019 at 01:12:01PM +0100, Peter Krempa wrote:
On Wed, Jan 16, 2019 at 09:52:42 +0000, Daniel Berrange wrote:
On Wed, Jan 16, 2019 at 10:42:10AM +0100, Peter Krempa wrote:
If a -drive has no image, using image properties makes qemu whine that they should not be used.
This patch stops formating cache/readonly/... for empty drives for the pre-blockdev syntax. Unfortunately those parameters can't be added later when inserting media, but on the other hand qemu will start with an empty drive.
So, IIUC, if you need to set cache/readonly/etc when inserting media, then the fix is simply to use a QEMU that supports blockdev?
Yes. With blockdev we setup the cache modes as the user wishes/configures. Currently AFAIK the user would not get the required mode anyways.
This should not be any problem for cdroms. Floppy drives might end up with incorrect cache mode.
If someone cares about data integrity of the floppy drives on host crash, I think they need to re-evaluate their decision making approach ;-P 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 :|
On Wed, Jan 16, 2019 at 12:15:51 +0000, Daniel Berrange wrote:
On Wed, Jan 16, 2019 at 01:12:01PM +0100, Peter Krempa wrote:
On Wed, Jan 16, 2019 at 09:52:42 +0000, Daniel Berrange wrote:
On Wed, Jan 16, 2019 at 10:42:10AM +0100, Peter Krempa wrote:
If a -drive has no image, using image properties makes qemu whine that they should not be used.
This patch stops formating cache/readonly/... for empty drives for the pre-blockdev syntax. Unfortunately those parameters can't be added later when inserting media, but on the other hand qemu will start with an empty drive.
So, IIUC, if you need to set cache/readonly/etc when inserting media, then the fix is simply to use a QEMU that supports blockdev?
Yes. With blockdev we setup the cache modes as the user wishes/configures. Currently AFAIK the user would not get the required mode anyways.
This should not be any problem for cdroms. Floppy drives might end up with incorrect cache mode.
If someone cares about data integrity of the floppy drives on host crash, I think they need to re-evaluate their decision making approach ;-P
So, is this patch acceptable? :)
On Wed, Jan 16, 2019 at 10:42:10AM +0100, Peter Krempa wrote:
If a -drive has no image, using image properties makes qemu whine that they should not be used.
This patch stops formating cache/readonly/... for empty drives for the pre-blockdev syntax. Unfortunately those parameters can't be added later when inserting media, but on the other hand qemu will start with an empty drive.
Since we already were able to start a VM with such config previously due to qemu ignoring them I've opted just to skip formatting them. Additionally with -blockdev support it will work as expected as the image properties will be formatted when adding the image itself which is not possible without it.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1651457
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 47 ++++++++++--------- tests/qemuxml2argvdata/disk-cdrom.args | 4 +- .../disk-cdrom.x86_64-2.12.0.args | 4 +- .../disk-cdrom.x86_64-latest.args | 4 +- 4 files changed, 30 insertions(+), 29 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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 :|
On Wed, Jan 16, 2019 at 10:42:08AM +0100, Peter Krempa wrote:
See patch 2/2 for explanation.
Peter Krempa (2): tests: qemuxml2argv: Add test case for empty CDROM with cache mode qemu: command: Don't format image properties for empty -drive
src/qemu/qemu_command.c | 47 ++++++++++--------- tests/qemuxml2argvdata/disk-cdrom.args | 6 ++- .../disk-cdrom.x86_64-2.12.0.args | 9 ++-- .../disk-cdrom.x86_64-latest.args | 9 ++-- tests/qemuxml2argvdata/disk-cdrom.xml | 6 +++ tests/qemuxml2xmloutdata/disk-cdrom.xml | 6 +++ 6 files changed, 52 insertions(+), 31 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa