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@xxxxxxxxxx> >>>> --- >>>> 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. > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
