Re: [libvirt] [PATCH v2] Perform disk config validity checking for attach-device config

On 08/07/2014 04:53 PM, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1078126 > > Using 'virsh attach-device --config' (or --persistent) to attach a > file backed lun device will succeed; however, subsequent domain restarts > will result in failure because the configuration of a file backed lun > is not supported. > > Although allowing 'illegal configurations' is something that can be > allowed, it may not be practical in this case. Generally, when attaching > a device to a domain means the domain must be running. A way around > this is using the --config (or --persistent) option. When an attach > is done to a running domain, a temporary configuration is modified > first followed by the live update. The live update will make a number > of disk validity checks when building the qemu command to attach the > disk. If any fail, then change is rejected. > > Rather than allow a potentially illegal combination, adjust the code > in the configuration path to make the same checks as the running path > will make with respect to disk validity checks. This way we avoid > having the potential for some subsequent start/reboot to fail because > an illegal combination was allowed. > > NB: The live path still checks the configuration since it is possible > to just do --live guest modification... > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > > Changes since V1: > http://www.redhat.com/archives/libvir-list/2014-August/msg00045.html > > Based on #virt IRC discussion this morning > - move the qemu_caps checking to only occur during the live check > - slightly change the name of the called routine > > src/qemu/qemu_command.c | 127 +++++++++++++++++++++++++++--------------------- > src/qemu/qemu_command.h | 2 + > src/qemu/qemu_driver.c | 2 + > 3 files changed, 76 insertions(+), 55 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 033a5a8..c1791d9 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -3230,6 +3230,73 @@ qemuGetDriveSourceString(virStorageSourcePtr src, > } > > > +/* Perform disk definition config validity checks */ > +int > +qemuBuildCheckDiskConfig(virDomainDiskDefPtr disk) Just a nitpick: 'Build' seems redundant here, how about just 'qemuCheckDiskConfig'? ACK either way. Jan