Re: [libvirt] [PATCH] qemu: Fix detection of drive readonly option
On Mon, Oct 25, 2010 at 03:56:58PM -0600, Eric Blake wrote:
On 10/25/2010 07:47 AM, Jiri Denemark wrote:
>So far, readonly=on option is used when qemu supports -device. However,
>there are qemu versions which support readonly option with -drive
>although they don't have support for -device.
>---
> src/qemu/qemu_conf.c | 12 ++-
> src/qemu/qemu_conf.h | 1 +
> tests/qemuhelpdata/kvm-83-rhel56 | 141
> ++++++++++++++++++++
> tests/qemuhelptest.c | 26 ++++
> ...qemuxml2argv-disk-drive-readonly-no-device.args | 1 +
> .../qemuxml2argv-disk-drive-readonly-no-device.xml | 31 +++++
> tests/qemuxml2argvtest.c | 5 +-
> 7 files changed, 214 insertions(+), 3 deletions(-)
> create mode 100644 tests/qemuhelpdata/kvm-83-rhel56
> create mode 100644
> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-no-device.args
> create mode 100644
> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-no-device.xml
ACK.
>+++ b/src/qemu/qemu_conf.h
>@@ -96,6 +96,7 @@ enum qemud_cmd_flags {
> QEMUD_CMD_FLAG_FSDEV = (1LL<< 40), /* -fstype filesystem
> passthrough */
> QEMUD_CMD_FLAG_NESTING = (1LL<< 41), /* -enable-nesting
> (SVM/VMX) */
> QEMUD_CMD_FLAG_NAME_PROCESS = (1LL<< 42), /* Is -name process=
> available */
>+ QEMUD_CMD_FLAG_DRIVE_READONLY = (1LL<< 43), /* -drive
>readonly=on|off */
It feels like we're burning through these flag bits rather quickly: 15
flags added since January of this year. While it doesn't affect your
patch, it means that at the current pace we have less than 18 months
before we have to add a second flag variable.
This isn't a problem. This is an internal ABI only, so we can change the
way we represent this anytime we like.
There is a bigger problem in this area. We run 'qemu -help' every time
we do any operation on the guest that requires these flags to be known.
This is bad because the 'qemu' binary may not be the same qemu binary
that we originally booted the guest with (ie RPM package upgrade). We
can thus detect the wrong flags potentially.
What I think we should do is define a bitset type 'virBitset'. Change
this to a plain enum, instead of a flag based enum, thus giving us
2^64 entries. We can then have APIs like virBitsetIsSet(num). The
virBitset data should be stored in the virDomainObjPtr instance at
the time of guest booting. We also need to persist it in the XML state
file /var/lib/libvirt/qemu/$guest.xml, as named enum entries, and reload
this at libvirtd restart.
I'm not likely to have any time to implement this in the forseeable
future though. Hopefully someone can do it before we use up the last
20 flags :-)
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|