On 03/07/2012 12:48 PM, Michal Privoznik wrote:
Some nits are generated during XML parse (e.g. MAC address of
an interface); However, with current implementation, if we
are plugging a device both to persistent and live config,
we parse given XML twice: first time for live, second for config.
This is wrong then as the second time we are not guaranteed
to generate same values as we did for the first time.
To prevent that we need to create a copy of DeviceDefPtr;
This is done through format/parse process instead of writing
functions for deep copy as it is easier to maintain:
adding new field to any virDomain*DefPtr doesn't require change
of copying function.
---
diff to v1:
-Eric's review included
src/conf/domain_conf.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 3 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 38 ++++++++++++++-----------
4 files changed, 95 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b994718..42cfbd5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14529,3 +14529,73 @@ virDomainNetFind(virDomainDefPtr def, const char *device)
return net;
}
I still think there should be a comment added here saying something like:
NB: virDomainDeviceDefCopy does a deep copy of only the parts of a
DeviceDef that are valid when just the flag VIR_DOMAIN_XML_INACTIVE is
set. This means that any part of the device xml that is conditionally
parsed/formatted based on some other flag being set (or on the INACTIVE
flag being reset) *will not* be copied to the destination. Caveat emptor.
+
+virDomainDeviceDefPtr
+virDomainDeviceDefCopy(virCapsPtr caps,
+ const virDomainDefPtr def,
+ virDomainDeviceDefPtr src)
Otherwise it looks like you've taken care of all of Eric's issues, and
seems clean, so ACK from me (conditional on adding a comment documenting
the limitations of the new copy function).