Re: [libvirt] [PATCH v2 05/11] conf: Add flag to regenerate genid for virDomainDefCopy
On 04/25/2018 03:42 AM, Peter Krempa wrote:
On Tue, Apr 24, 2018 at 14:13:15 -0400, John Ferlan wrote:
>
>
> On 04/24/2018 03:24 AM, Peter Krempa wrote:
>> On Mon, Apr 23, 2018 at 19:59:59 -0400, John Ferlan wrote:
>>> Add the VIR_DOMAIN_DEF_COPY_NEWGENID to indicate that the generated
>>> domain definition should adjust the genid value before returning the
>>> @def to the caller.
>>>
>>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>>> ---
>>> src/conf/domain_conf.c | 12 ++++++++++++
>>> src/conf/domain_conf.h | 5 +++++
>>> src/qemu/qemu_driver.c | 3 ++-
>>> src/test/test_driver.c | 3 ++-
>>> 4 files changed, 21 insertions(+), 2 deletions(-)
>>
>> IMO all this code should not be necessary. Code paths which knowingly
>> need to change the GENID should do so explicitly rather than pulling the
>> logic here.
>>
>
> Honestly, I found it easier to have the code in one place rather than
> cut-copy-paste in two places... But I can move it out.
The problem is that the copying of domain definition should not attempt
any smarts. The caller expects a copy, so it should be the same. If the
caller wishes to use the definition in a different way it needs to be
modified afterwards.
That ship already sailed by passing migratable, hence why it seemed fine
to use the function. But like I said, I'll move it out.
Although from this discussion it appears parseOpaque could be
unnecessary - all callers pass NULL in the 4th parameter. Still it could
be well enough hidden in some other way...
John
> However, I may have forgotten a transition though w/r/t domain
copy.
> Unfortunately clone could be a bugger since it uses parse and format,
> but if startup always generates one, then that's perhaps covered.
Copying of the domain definition should always copy the GUID along with
the flag whether it was autogenerated or not. You may need a XML
attribute for that. Otherwise you will not be able to differentiate the
situation when the GUID was provided by the user, or when it was
autogenerated but needs to be used as-is.