Re: [libvirt] [PATCH] virDomainDetachDeviceFlags: Clarify update semantics

On 08/27/2018 09:50 AM, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1621910 > FWIW: my thoughts w/r/t "alias" (in particular) and the bz - since libvirt creates the alias for the live XML and does not extend that alias to the config XML that means I can see just cause for the client to believe libvirt should handle supplying the alias for the update. I thought that was one of the factors behind creating "ua-" prefixes for user aliases. IOW: User supplied an alias, so use it; otherwise, use what libvirt creates. One could say something similar for <address> since we create it; however, the difference there is that the <address> is written to the config XML for the next time. While I agree in general with supplying a "robust" XML model in order to be certain that we'd be updating the "correct" device and not just the "first device" found, I'm not convinced that by just documenting it the "problem" is fixed and the continuing conversation in the bz since you posted this patch is, well, interesting. In a way it's bringing up the reasons that we (libvirt) find it difficult to ensure we're making the right update without adding "too much" code. Move the problem up the stack! > When users want to update a path to a CDROM they tend to > construct a very minimal XML and feed the API with it. This is > not a good practice as it breaks the assumptions the API is built > on. Most notably, leaving an element out should be treated as a > request for removal of the corresponding setting. Just like > leaving out <bandwidth/> clears out any QoS previously set. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/libvirt-domain.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > index ef460277f7..bd8ca6eff2 100644 > --- a/src/libvirt-domain.c > +++ b/src/libvirt-domain.c > @@ -8318,6 +8318,14 @@ virDomainDetachDeviceFlags(virDomainPtr domain, > * media, altering the graphics configuration such as password, > * reconfiguring the NIC device backend connectivity, etc. > * > + * The supplied XML description of the device should contain all > + * the information that are found in corresponding domain XML. s/that are/that is/ > + * Leaving out any piece of information is treated as request for s/is treated/may be treated/ Wouldn't that be hypervisor dependent? Sure most seem to call virDomainDeviceDefParse with PARSE_INACTIVE | PARSE_SKIP_VALIDATE

> + * its removal, which may be denied. For instance, when users > + * want to change CDROM media only for live XML, they must > + * provide live disk XML as found in corresponding live domain > + * XML with only the disk path changed. Seems that's easier written than done in practice! In particular the means by which each device is uniquely identified for the live guest.

For QEMU that's the alias.

Is that true for other hypervisors? I think that's where the argument starts to fall apart that one must provide full XML.