Re: [libvirt] [PATCH v4] virsh: Increase device-detach intelligence
On 11/22/2011 03:26 AM, Michal Privoznik wrote:
From: Michal Prívozník <mprivozn(a)redhat.com>
Up to now users have to give a full XML description on input when
device-detaching. If they omitted something it lead to unclear
error messages (like generated MAC wasn't found, etc.).
With this patch users can specify only those information which
specify one device sufficiently precise. Remaining information is
completed from domain.
---
diff to v3:
- Eric's review:
https://www.redhat.com/archives/libvir-list/2011-September/msg00472.html
Been a while since we last looked at this :) Looks like you correctly
implemented my algorithm suggestions from the v3 comments.
+/**
+ * Check if n1 is superset of n2, meaning n1 contains all elements and
+ * attributes as n2 at least. Including children.
+ * @n1 first node
+ * @n2 second node
+ * return 1 in case n1 covers n2, 0 otherwise.
+ */
+static bool
Comment needs a tweak:
returns true in case n1 covers n2, false otherwise
+
+ n1_child_size = xmlChildElementCount(n1);
+ if (!n1_child_size) {
+ return !xmlChildElementCount(n2);
+ }
Rewriting this chunk will give you a minor optimization in more
situations. For example, if n1 has two children but n2 has three
children (to be a subset, n2 cannot have any more children than n1):
n1_child_size = xmlChildElementCount(n1);
n2_child_size = xmlChildElementCount(n2);
if (n1_child_size < n2_child_size)
return false;
+static int
+vshCompleteXMLFromDomain(vshControl *ctl, virDomainPtr dom, char *oldXML,
+ char **newXML) {
We haven't always been consistent, but generally the open { of a
function appears on column 1 of a new line (same comment for
vshNodeIsSuperset).
+ int funcRet = -1;
+ char *domXML = NULL;
+ xmlDocPtr domDoc = NULL, devDoc = NULL;
+ xmlNodePtr node = NULL;
+ xmlXPathContextPtr domCtxt = NULL, devCtxt = NULL;
+ xmlNodePtr *devices = NULL;
+ xmlSaveCtxtPtr sctxt = NULL;
+ int devices_size;
+ char *xpath;
To avoid crash on early error, this must be:
char *xpath = NULL;
+ buf = xmlBufferCreate();
+ if (!buf) {
+ vshError(ctl, "%s", _("out of memory"));
+ goto cleanup;
+ }
+
+ /* Get all possible devices */
+ virAsprintf(&xpath, "/domain/devices/%s", node->name);
+ if (!xpath) {
+ virReportOOMError();
+ goto cleanup;
since xpath is allocated on some, but not all paths, into cleanup.
+
+cleanup:
+ xmlBufferFree(buf);
+ VIR_FREE(devices);
+ xmlXPathFreeContext(devCtxt);
+ xmlXPathFreeContext(domCtxt);
+ xmlFreeDoc(devDoc);
+ xmlFreeDoc(domDoc);
+ VIR_FREE(domXML);
+ return funcRet;
Mem leak if you don't have:
VIR_FREE(xpath);
ACK with those problems fixed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)