On 07/28/2015 09:56 AM, Peter Krempa wrote:
Since the code is changing the source image path by modifying the
existing XML snippet the backingStore element does not get dropped.
The element is ignored though but it might start being used in the
future.
Before this patch, you'd get:
$ virsh change-media --eject --print-xml 10 hdc
disk type=file device=cdrom
driver name=qemu type=qcow2/
backingStore type=file index=1
format type=qcow2/
source file=/var/lib/libvirt/images/vm.1436949097/
backingStore/
/backingStore
target dev=hdc bus=ide/
...
/disk
After:
$ virsh change-media --eject --print-xml 10 hdc
disk type=file device=cdrom
driver name=qemu type=qcow2/
target dev=hdc bus=ide/
...
/disk
---
tools/virsh-domain.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
Code will need to be updated with recent virsh changes... I'm not quite
sure I understand what's being done or why it's necessary from the
commit explanation.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a61656f..d4d6987 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11415,6 +11415,7 @@ vshUpdateDiskXML(xmlNodePtr disk_node,
vshUpdateDiskXMLType type)
{
xmlNodePtr source = NULL;
+xmlNodePtr child;
char *device_type = NULL;
char *ret = NULL;
@@ -11430,10 +11431,16 @@ vshUpdateDiskXML(xmlNodePtr disk_node,
}
/* find the current source subelement */
-for (source = disk_node-children; source; source = source-next) {
-if (source-type == XML_ELEMENT_NODE
-xmlStrEqual(source-name, BAD_CAST source))
-break;
+for (child = disk_node-children; child; child = child-next) {
+if (child-type == XML_ELEMENT_NODE
+xmlStrEqual(child-name, BAD_CAST source))
+source = child;
+
+if (child-type == XML_ELEMENT_NODE
+xmlStrEqual(child-name, BAD_CAST backingStore)) {
+xmlUnlinkNode(child);
+xmlFreeNode(child);
+}
here you've freed child and from how I read the existing code when
'source' goes through the same process, there's a 'source = NULL;'
following, so it seems there should be a child = NULL; added.
Without that, would the next iteration of the loop reference something
that was free'd? That is child is local to this function, it's not
passed by reference, thus a subsequent ; child; child = child-next
may be by chance pointing to something valid, but could be pointing to
something invalid too.
I think with the merge with latest changes and the child = NULL this is
ACK-able.
John
}
if (type == VSH_UPDATE_DISK_XML_EJECT) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
