Re: [PATCH] virDomainDetachDeviceFlags: Clarify update semantics

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux