Re: [PATCH v2 4/5] domain: Define explicit flags for saved image xml

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

 



On 2/19/19 1:38 PM, John Ferlan wrote:
> 
> 
> On 2/14/19 4:29 PM, Eric Blake wrote:
>> Commit d2a929d4 (0.9.4) defined virDomainSaveImageGetXMLDesc()'s use
>> of @flags as a subset of virDomainXMLFlags, documenting that 2 of the
>> 3 flags defined at the time would never be valid.  Later, commit
>> 28f8dfdc (1.0.0) introduced a new flag, VIR_DOMAIN_XML_MIGRATABLE, but
>> did not adjust the save image documentation to declare it as invalid.
>> Later, commit a67e3872 (3.7.0) blindly copied and pasted the same text
>> into virDomainManagedSaveGetXMLDesc.
>>
>> However, since the flag is not accepted as valid by any of the
>> drivers (remote is just passthrough; and qemu is the only supporting
>> driver for either API, with support for just VIR_DOMAIN_XML_SECURE),
>> it is easier to just define an explicit set of supported flags
>> directly related to the save image API rather than trying to borrow
>> from live domain API, and risking confusion if even more domain flags
>> are added later (in fact, I have an upcoming patch that plans to add
>> a new flag to virDomainGetXMLDesc that makes no sense for saved
>> images).  We may someday decide that saved images need to support the
>> _MIGRATABLE flag, as it is possible to load a saved image with a
>> different version of libvirt than the one that created it, but that
>> can be a separate patch if it is ever needed.  Meanwhile, it DOES make
>> sense to reuse the same flags for SaveImage and for ManagedSave (since
>> ManagedSave is really just sugar for creating a normal SaveImage in a
>> location controlled by libvirt instead of by the user).
>>
>> There is no API or ABI impact (since we purposefully used unsigned int
>> rather than an enum type in public API, and since the new flag name
>> carries the same value as the old reused name).
>>
>> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
>> ---
>>  include/libvirt/libvirt-domain.h |  5 +++++
>>  src/libvirt-domain.c             | 14 ++++++--------
>>  src/qemu/qemu_driver.c           |  4 ++--
>>  src/remote/remote_protocol.x     |  4 ++--
>>  4 files changed, 15 insertions(+), 12 deletions(-)
>>
> 
> Although it could be pointed out that the comment lines in qemu_driver
> above each flag change is moot.

Oh, good point. I'll drop those comments.

> 
> Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
> 
> John
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[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