[libvirt] [PATCH v2 05/11] conf: Add flag to regenerate genid for virDomainDefCopy

[John Ferlan]

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 at 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.
>