[libvirt] [PATCH] virDomainDetachDeviceFlags: Clarify update semantics

Erik Skultety eskultet at redhat.com
Wed Aug 29 13:34:41 UTC 2018


On Mon, Aug 27, 2018 at 03:50:20PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1621910
>
> 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 at redhat.com>
> ---
>  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  /that /
s/are found/is found/ (information is uncountable)
s/in (corresponding)/in the \1/

> + * Leaving out any piece of information is treated as request for

s/as request/as a request/

> + * 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

s/in /in the/

> + * XML with only the disk path changed.

I agree that such a laziness only leads to an inconsistent user experience
where libvirt has no idea whether the user is just truly lazy to supply all the
necessary information and therefore we're supposed to preserve such setting or
they actually want to remove the setting.

Reviewed-by: Erik Skultety <eskultet at redhat.com>




More information about the libvir-list mailing list