[libvirt] [PATCH] qemuDomainAttach: Initialize pidfile variable
If parsing qemu command line fails (e.g. because of non-existing process number supplied), we jump to cleanup label where we free pidfile. Therefore it needs to be initialized. Otherwise we free random pointer. --- Pushing under trivial rule. src/qemu/qemu_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2e6f3e4..5588d93 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10179,7 +10179,7 @@ static virDomainPtr qemuDomainAttach(virConnectPtr conn, virDomainPtr dom = NULL; virDomainChrSourceDefPtr monConfig = NULL; bool monJSON = false; -char *pidfile; +char *pidfile = NULL; virCheckFlags(0, NULL); -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Qemu/KVM guest boots 2x slower with vhost_net
On 10/05/11 01:12, Reeted wrote: . I found that virtual machines in my host booted 2x slower ... to the vhost_net presence ... Just a small update, Firstly: I cannot reproduce any slowness after boot by doing: # time /etc/init.d/chrony restart Restarting time daemon: Starting /usr/sbin/chronyd... chronyd is running and online. real0m3.022s user0m0.000s sys 0m0.000s since this is a network service I expected it to show the problem, but it doesn't. It takes exactly same time with and without vhost_net. Secondly, vhost_net appears to work correctly, because I have performed a NPtcp performance test between two guests in the same host, and these are the results: vhost_net deactivated for both hosts: NPtcp -h 192.168.7.81 Send and receive buffers are 16384 and 87380 bytes (A bug in Linux doubles the requested buffer sizes) Now starting the main loop 0: 1 bytes917 times -- 0.08 Mbps in 92.07 usec 1: 2 bytes 1086 times -- 0.18 Mbps in 86.04 usec 2: 3 bytes 1162 times -- 0.27 Mbps in 85.08 usec 3: 4 bytes783 times -- 0.36 Mbps in 85.34 usec 4: 6 bytes878 times -- 0.54 Mbps in 85.42 usec 5: 8 bytes585 times -- 0.72 Mbps in 85.31 usec 6: 12 bytes732 times -- 1.07 Mbps in 85.52 usec 7: 13 bytes487 times -- 1.16 Mbps in 85.52 usec 8: 16 bytes539 times -- 1.43 Mbps in 85.26 usec 9: 19 bytes659 times -- 1.70 Mbps in 85.43 usec 10: 21 bytes739 times -- 1.77 Mbps in 90.71 usec 11: 24 bytes734 times -- 2.13 Mbps in 86.13 usec 12: 27 bytes822 times -- 2.22 Mbps in 92.80 usec 13: 29 bytes478 times -- 2.35 Mbps in 94.02 usec 14: 32 bytes513 times -- 2.60 Mbps in 93.75 usec 15: 35 bytes566 times -- 3.15 Mbps in 84.77 usec 16: 45 bytes674 times -- 4.01 Mbps in 85.56 usec 17: 48 bytes779 times -- 4.32 Mbps in 84.70 usec 18: 51 bytes811 times -- 4.61 Mbps in 84.32 usec 19: 61 bytes465 times -- 5.08 Mbps in 91.57 usec 20: 64 bytes537 times -- 5.22 Mbps in 93.46 usec 21: 67 bytes551 times -- 5.73 Mbps in 89.20 usec 22: 93 bytes602 times -- 8.28 Mbps in 85.73 usec 23: 96 bytes777 times -- 8.45 Mbps in 86.70 usec 24: 99 bytes780 times -- 8.71 Mbps in 86.72 usec 25: 125 bytes419 times -- 11.06 Mbps in 86.25 usec 26: 128 bytes575 times -- 11.38 Mbps in 85.80 usec 27: 131 bytes591 times -- 11.60 Mbps in 86.17 usec 28: 189 bytes602 times -- 16.55 Mbps in 87.14 usec 29: 192 bytes765 times -- 16.80 Mbps in 87.19 usec 30: 195 bytes770 times -- 17.11 Mbps in 86.94 usec 31: 253 bytes401 times -- 22.04 Mbps in 87.59 usec 32: 256 bytes568 times -- 22.64 Mbps in 86.25 usec 33: 259 bytes584 times -- 22.68 Mbps in 87.12 usec 34: 381 bytes585 times -- 33.19 Mbps in 87.58 usec 35: 384 bytes761 times -- 33.54 Mbps in 87.36 usec 36: 387 bytes766 times -- 33.91 Mbps in 87.08 usec 37: 509 bytes391 times -- 44.23 Mbps in 87.80 usec 38: 512 bytes568 times -- 44.70 Mbps in 87.39 usec 39: 515 bytes574 times -- 45.21 Mbps in 86.90 usec 40: 765 bytes580 times -- 66.05 Mbps in 88.36 usec 41: 768 bytes754 times -- 66.73 Mbps in 87.81 usec 42: 771 bytes760 times -- 67.02 Mbps in 87.77 usec 43:1021 bytes384 times -- 88.04 Mbps in 88.48 usec 44:1024 bytes564 times -- 88.30 Mbps in 88.48 usec 45:1027 bytes566 times -- 88.63 Mbps in 88.40 usec 46:1533 bytes568 times -- 71.75 Mbps in 163.00 usec 47:1536 bytes408 times -- 72.11 Mbps in 162.51 usec 48:1539 bytes410 times -- 71.71 Mbps in 163.75 usec 49:2045 bytes204 times -- 95.40 Mbps in 163.55 usec 50:2048 bytes305 times -- 95.26 Mbps in 164.02 usec 51:2051 bytes305 times -- 95.33 Mbps in 164.14 usec 52:3069 bytes305 times --141.16 Mbps in 165.87 usec 53:3072 bytes401 times --142.19 Mbps in 164.83 usec 54:3075 bytes404 times --150.68 Mbps in 155.70 usec 55:4093 bytes214 times --192.36 Mbps in 162.33 usec 56:4096 bytes307 times --193.21 Mbps in 161.74 usec
Re: [libvirt] [PATCH 6/7] Rewrite all the DTrace/SystemTAP probing
On Fri, Oct 07, 2011 at 12:01:18PM -0600, Eric Blake wrote: On 10/07/2011 09:56 AM, Daniel P. Berrange wrote: [...] It's big, but useful, and I think that we can clean up the fallout as separate patches as we port it to more compilation environments (VPATH, older systemtap headers, etc.). I'm okay giving ACK. It would be good to test for portability, like old systemtap versions as this used to be a problem, but looks fine to me too :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] snapshot: sort snapshot-list --tree
On Fri, Oct 07, 2011 at 04:35:28PM -0600, Eric Blake wrote:
Otherwise, the results are not repeatable.
* tools/virsh.c (cmdSnapshotList): Print tree in predictable order.
---
tools/virsh.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 9532bc3..20b3dc5 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13209,6 +13209,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (actual 0)
goto cleanup;
+qsort(names[0], actual, sizeof(char*), namesorter);
+
if (tree) {
char indentBuf[INDENT_BUFLEN];
char **parents = vshMalloc(ctl, sizeof(char *) * actual);
@@ -13245,8 +13247,6 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
ret = true;
goto cleanup;
} else {
-qsort(names[0], actual, sizeof(char*), namesorter);
-
for (i = 0; i actual; i++) {
/* free up memory from previous iterations of the loop */
VIR_FREE(parent);
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] snapshot: avoid accidental renames with snapshot-edit
On Fri, Oct 07, 2011 at 04:35:29PM -0600, Eric Blake wrote:
I was a bit surprised that 'virsh snapshot-edit dom name' silently
allowed me to clone things, while still telling me the old name,
especially since other commands like 'virsh edit dom' reject rename
attempts (*). This fixes things to be more explicit.
(*) Technically, 'virsh edit dom' relies on virDomainDefineXML
behavior, which rejects attempts to mix a new name with existing
uuid or new uuid with existing name, but you can create a new
domain by changing both uuid and name. On the other hand, while
snapshot-edit --clone is a true clone, creating a new domain
would also have to decide whether to clone snapshot metadata,
managed save, and any other secondary data related to the domain.
Domain renames are not trivial either.
* tools/virsh.c (cmdSnapshotEdit): Add --rename, --clone.
* tools/virsh.pod (snapshot-edit): Document them.
---
tools/virsh.c | 45 -
tools/virsh.pod |6 ++
2 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 20b3dc5..7151694 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12830,6 +12830,8 @@ static const vshCmdOptDef opts_snapshot_edit[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
{snapshotname, VSH_OT_DATA, VSH_OFLAG_REQ, N_(snapshot name)},
{current, VSH_OT_BOOL, 0, N_(also set edited snapshot as current)},
+{rename, VSH_OT_BOOL, 0, N_(allow renaming an existing snapshot)},
+{clone, VSH_OT_BOOL, 0, N_(allow cloning to new name)},
{NULL, 0, 0, NULL}
};
@@ -12838,13 +12840,23 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
virDomainSnapshotPtr snapshot = NULL;
+virDomainSnapshotPtr edited = NULL;
const char *name;
+const char *edited_name;
bool ret = false;
char *tmp = NULL;
char *doc = NULL;
char *doc_edited = NULL;
unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE;
unsigned int define_flags = VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
+bool rename_okay = vshCommandOptBool(cmd, rename);
+bool clone_okay = vshCommandOptBool(cmd, clone);
+
+if (rename_okay clone_okay) {
+vshError(ctl, %s,
+ _(--rename and --clone are mutually exclusive));
+return false;
+}
if (vshCommandOptBool(cmd, current))
define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT;
@@ -12867,8 +12879,6 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
doc = virDomainSnapshotGetXMLDesc(snapshot, getxml_flags);
if (!doc)
goto cleanup;
-virDomainSnapshotFree(snapshot);
-snapshot = NULL;
/* strstr is safe here, since xml came from libvirt API and not user */
if (strstr(doc, statedisk-snapshot/state))
@@ -12899,13 +12909,36 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
}
/* Everything checks out, so redefine the xml. */
-snapshot = virDomainSnapshotCreateXML(dom, doc_edited, define_flags);
-if (!snapshot) {
+edited = virDomainSnapshotCreateXML(dom, doc_edited, define_flags);
+if (!edited) {
vshError(ctl, _(Failed to update %s), name);
goto cleanup;
}
-vshPrint(ctl, _(Snapshot %s edited.\n), name);
+edited_name = virDomainSnapshotGetName(edited);
+if (STREQ(name, edited_name)) {
+vshPrint(ctl, _(Snapshot %s edited.\n), name);
+} else if (clone_okay) {
+vshPrint(ctl, _(Snapshot %s cloned to %s.\n), name,
+ edited_name);
+} else {
+unsigned int delete_flags;
+
+delete_flags = VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY;
+if (virDomainSnapshotDelete(rename_okay ? snapshot : edited,
+delete_flags) 0) {
+virshReportError(ctl);
+vshError(ctl, _(Failed to clean up %s),
+ rename_okay ? name : edited_name);
+goto cleanup;
+}
+if (!rename_okay) {
+vshError(ctl, _(Must use --rename or --clone to change %s to
%s),
+ name, edited_name);
+goto cleanup;
+}
+}
+
ret = true;
cleanup:
@@ -12917,6 +12950,8 @@ cleanup:
}
if (snapshot)
virDomainSnapshotFree(snapshot);
+if (edited)
+virDomainSnapshotFree(edited);
if (dom)
virDomainFree(dom);
return ret;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 1f7c76f..3351fe0 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1952,6 +1952,7 @@ With Isnapshotname, this is a request to make the
existing named
snapshot become the current snapshot, without reverting the domain.
=item Bsnapshot-edit Idomain Isnapshotname [I--current]
+{[I--rename] | [I--clone]}
Edit the XML configuration file for Isnapshotname of
Re: [libvirt] [PATCH 0/4] snapshot: optimize qemu relationship traversal
On Fri, Oct 07, 2011 at 06:05:53PM -0600, Eric Blake wrote: After working with ESX and VBox, I realized that a tweak to how snapshot relationships are stored would speed things up from O(n^2) to O(n). The hash table must be kept for name lookup, but the So we went from 0(n^3) to O(n), sounds good :-) speedup comes by tracking relationships with each snapshot rather than crawling the entire hash table rebuilding those relations for every operation that needs particular relations. As long as we are sure there is no risk of stale relationship metadata, this sounds proper ... Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] snapshot: framework for more efficient relation traversal
On Fri, Oct 07, 2011 at 06:05:54PM -0600, Eric Blake wrote:
No one was using virDomainSnapshotHasChildren, but that was an
O(n) function. Exposing and tracking a bit more metadata for each
snapshot will allow the same query to be made with an O(1) query
of the member field. For single snapshot operations (create,
delete), callers can be trusted to maintain the metadata themselves,
but for reloading, we can't compute parents as we go since there
is no guarantee which order things are visited in, so we also
provide a function to refresh the relationships, and which can
be used to detect if the user has ignored our warnings and been
directly modifying files in /var/lib/libvirt/qemu/snapshot. This
patch only adds metadata; later patches will actually use it.
This layout intentionally hardcodes the size of each snapshot,
by tracking sibling pointers, rather than having to deal with
the headache of yet more memory management by directly sticking
a child[] on each parent.
Okay, so you're using a 'first child'. But why try to maintain
'nchildren' since you can't get the list in 0(1) anyway and
need to walk the sibling. It's just redundant data to me, but maybe
I didn't understood the use :-)
* src/conf/domain_conf.h (_virDomainSnapshotObj)
(_virDomainSnapshotObjList): Add members.
(virDomainSnapshotUpdateRelations, virDomainSnapshotDropParent):
New prototypes.
(virDomainSnapshotHasChildren): Delete.
* src/conf/domain_conf.c (virDomainSnapshotSetRelations)
(virDomainSnapshotUpdateRelations, virDomainSnapshotDropParent):
New functions.
(virDomainSnapshotHasChildren): Drop unused function.
* src/libvirt_private.syms (domain_conf): Update exports.
---
src/conf/domain_conf.c | 108
+++---
src/conf/domain_conf.h | 13 +-
src/libvirt_private.syms |3 +-
3 files changed, 114 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b9ddf26..d4e127d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12312,13 +12312,6 @@
virDomainSnapshotForEachChild(virDomainSnapshotObjListPtr snapshots,
return act.number;
}
-
-int virDomainSnapshotHasChildren(virDomainSnapshotObjPtr snap,
- virDomainSnapshotObjListPtr snapshots)
-{
-return virDomainSnapshotForEachChild(snapshots, snap, NULL, NULL);
-}
-
typedef enum {
MARK_NONE, /* No relation determined yet */
MARK_DESCENDANT, /* Descendant of target */
@@ -12423,6 +12416,107 @@
virDomainSnapshotForEachDescendant(virDomainSnapshotObjListPtr snapshots,
return act.number;
}
+
+struct snapshot_set_relation {
+virDomainSnapshotObjListPtr snapshots;
+int err;
+};
+
The following function isn't trivial, worth adding a comment about
expected use.
+static void
+virDomainSnapshotSetRelations(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *data)
+{
+virDomainSnapshotObjPtr obj = payload;
+struct snapshot_set_relation *curr = data;
+virDomainSnapshotObjPtr tmp;
+
+if (obj-def-parent) {
+obj-parent = virDomainSnapshotFindByName(curr-snapshots,
+ obj-def-parent);
+if (!obj-parent) {
+curr-err = -1;
+VIR_WARN(snapshot %s lacks parent, obj-def-name);
+} else {
+tmp = obj-parent;
+while (tmp) {
+if (tmp == obj) {
+curr-err = -1;
+obj-parent = NULL;
+VIR_WARN(snapshot %s in circular chain,
obj-def-name);
+break;
+}
+tmp = tmp-parent;
+}
+if (!tmp) {
+obj-parent-nchildren++;
+obj-sibling = obj-parent-first_child;
+obj-parent-first_child = obj;
+}
+}
+} else {
+curr-snapshots-nroots++;
+obj-sibling = curr-snapshots-first_root;
+curr-snapshots-first_root = obj;
+}
+}
+
+/* Populate parent link and child count of all snapshots, with all
+ * relations starting as 0/NULL. Return 0 on success, -1 if a parent
+ * is missing or if a circular relationship was requested. */
+int
+virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots)
+{
+struct snapshot_set_relation act = { snapshots, 0 };
+
+virHashForEach(snapshots-objs, virDomainSnapshotSetRelations, act);
+return act.err;
+}
+
+/* Prepare to reparent or delete snapshot, by removing it from its
+ * current listed parent. Note that when bulk removing all children
+ * of a parent, it is faster to just 0 the count rather than calling
+ * this function on each child. */
+void
+virDomainSnapshotDropParent(virDomainSnapshotObjListPtr snapshots,
+
Re: [libvirt] [PATCH 2/4] snapshot: track qemu snapshot relations
On Fri, Oct 07, 2011 at 06:05:55PM -0600, Eric Blake wrote:
Maintain the parent/child relationships of all qemu snapshots.
* src/qemu/qemu_driver.c (qemuDomainSnapshotLoad): Populate
relationships after loading.
(qemuDomainSnapshotCreateXML): Set relations on creation; tweak
redefinition to reuse existing object.
(qemuDomainSnapshotReparentChildren, qemuDomainSnapshotDelete):
Clear relations on delete.
---
src/qemu/qemu_driver.c | 72 ++-
1 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5db67b8..501a3fc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -376,6 +376,10 @@ static void qemuDomainSnapshotLoad(void *payload,
vm-current_snapshot = NULL;
}
+if (virDomainSnapshotUpdateRelations(vm-snapshots) 0)
+VIR_ERROR(_(Snapshots have inconsistent relations for domain %s),
+ vm-def-name);
+
Hum, so we error out but continue with possibly inconsistent metadata,
isn't that risky ? What would be the cost or failing the load here and
consequences of lack of metadata ?
/* FIXME: qemu keeps internal track of snapshots. We can get access
* to this info via the info snapshots monitor command for running
* domains, or via qemu-img snapshot -l for shutoff domains. It would
@@ -9148,6 +9152,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
virDomainSnapshotDefPtr def = NULL;
bool update_current = true;
unsigned int parse_flags = 0;
+virDomainSnapshotObjPtr other = NULL;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT |
@@ -9190,8 +9195,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
goto cleanup;
if (flags VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) {
-virDomainSnapshotObjPtr other = NULL;
-
/* Prevent circular chains */
if (def-parent) {
if (STREQ(def-name, def-parent)) {
@@ -9267,7 +9270,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
update_current = true;
vm-current_snapshot = NULL;
}
-virDomainSnapshotObjListRemove(vm-snapshots, other);
+/* Drop and rebuild the parent relationship, but keep all
+ * child relations by reusing snap. */
+virDomainSnapshotDropParent(vm-snapshots, other);
+virDomainSnapshotDefFree(other-def);
+other-def = NULL;
+snap = other;
}
if (def-state == VIR_DOMAIN_DISK_SNAPSHOT def-dom) {
if (virDomainSnapshotAlignDisks(def,
@@ -9309,7 +9317,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
}
}
-if (!(snap = virDomainSnapshotAssignDef(vm-snapshots, def)))
+if (snap)
+snap-def = def;
+else if (!(snap = virDomainSnapshotAssignDef(vm-snapshots, def)))
goto cleanup;
def = NULL;
@@ -9366,11 +9376,25 @@ cleanup:
if (vm) {
if (snapshot !(flags VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
if (qemuDomainSnapshotWriteMetadata(vm, snap,
-driver-snapshotDir) 0)
+driver-snapshotDir) 0) {
VIR_WARN(unable to save metadata for snapshot %s,
snap-def-name);
-else if (update_current)
-vm-current_snapshot = snap;
+} else {
+if (update_current)
+vm-current_snapshot = snap;
+if (snap-def-parent) {
+other = virDomainSnapshotFindByName(vm-snapshots,
+snap-def-parent);
+snap-parent = other;
+other-nchildren++;
+snap-sibling = other-first_child;
+other-first_child = snap;
+} else {
+vm-snapshots.nroots++;
+snap-sibling = vm-snapshots.first_root;
+vm-snapshots.first_root = snap;
+}
+}
} else if (snap) {
virDomainSnapshotObjListRemove(vm-snapshots, snap);
}
@@ -10062,9 +10086,10 @@ cleanup:
struct snap_reparent {
struct qemud_driver *driver;
-const char *parent;
+virDomainSnapshotObjPtr parent;
virDomainObjPtr vm;
int err;
+virDomainSnapshotObjPtr last;
};
static void
@@ -10080,9 +10105,10 @@ qemuDomainSnapshotReparentChildren(void *payload,
}
VIR_FREE(snap-def-parent);
+snap-parent = rep-parent;
-if (rep-parent != NULL) {
-snap-def-parent = strdup(rep-parent);
+if (rep-parent) {
+snap-def-parent = strdup(rep-parent-def-name);
Re: [libvirt] [PATCH 3/4] snapshot: take advantage of new relations
On Fri, Oct 07, 2011 at 06:05:56PM -0600, Eric Blake wrote:
Among other improvements, virDomainSnapshotForEachDescendant is
changed from iterative O(n^2) to recursive O(n). A bit better
than the O(n^3) implementation in virsh snapshot-list!
* src/conf/domain_conf.c (virDomainSnapshotObjListNum)
(virDomainSnapshotObjListNumFrom)
(virDomainSnapshotObjeListGetNames, virDomainSnapshotForEachChild)
(virDomainSnapshotForEachDescendant): Optimize.
(virDomainSnapshotActOnDescendant): Tweak.
(virDomainSnapshotActOnChild, virDomainSnapshotMarkDescendant):
Delete, now that they are unused.
---
src/conf/domain_conf.c | 148
++--
1 files changed, 43 insertions(+), 105 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d4e127d..e77257a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12163,10 +12163,21 @@ int
virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
char **const names, int maxnames,
unsigned int flags)
{
-struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, flags };
+struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, 0 };
int i;
-virHashForEach(snapshots-objs, virDomainSnapshotObjListCopyNames,
data);
+data.flags = flags ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+
+if (!(flags VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
+virHashForEach(snapshots-objs, virDomainSnapshotObjListCopyNames,
+ data);
+} else {
+virDomainSnapshotObjPtr root = snapshots-first_root;
+while (root) {
+virDomainSnapshotObjListCopyNames(root, root-def-name, data);
+root = root-sibling;
+}
+}
if (data.oom) {
virReportOOMError();
goto cleanup;
@@ -12231,9 +12242,21 @@ static void virDomainSnapshotObjListCount(void
*payload,
int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
unsigned int flags)
{
-struct virDomainSnapshotNumData data = { 0, flags };
+struct virDomainSnapshotNumData data = { 0, 0 };
-virHashForEach(snapshots-objs, virDomainSnapshotObjListCount, data);
+data.flags = flags ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+
+if (!(flags VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
+virHashForEach(snapshots-objs, virDomainSnapshotObjListCount,
data);
+} else if (data.flags) {
+virDomainSnapshotObjPtr root = snapshots-first_root;
+while (root) {
+virDomainSnapshotObjListCount(root, root-def-name, data);
+root = root-sibling;
+}
I was just wondering if a data structure with a meta-root snapshot
and unifying first_root as a first_children on that meta-root wouldn't
lead to simpler code overall.
+} else {
+data.count = snapshots-nroots;
+}
return data.count;
}
@@ -12251,9 +12274,11 @@
virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
virDomainSnapshotForEachDescendant(snapshots, snapshot,
virDomainSnapshotObjListCount,
data);
-else
+else if (data.flags)
virDomainSnapshotForEachChild(snapshots, snapshot,
virDomainSnapshotObjListCount, data);
+else
+data.count = snapshot-nchildren;
return data.count;
}
@@ -12271,97 +12296,23 @@ void
virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
virHashRemoveEntry(snapshots-objs, snapshot-def-name);
}
-struct snapshot_act_on_child {
-char *parent;
-int number;
-virHashIterator iter;
-void *data;
-};
-
-static void
-virDomainSnapshotActOnChild(void *payload,
-const void *name,
-void *data)
-{
-virDomainSnapshotObjPtr obj = payload;
-struct snapshot_act_on_child *curr = data;
-
-if (obj-def-parent STREQ(curr-parent, obj-def-parent)) {
-curr-number++;
-if (curr-iter)
-(curr-iter)(payload, name, curr-data);
-}
-}
-
/* Run iter(data) on all direct children of snapshot, while ignoring all
* other entries in snapshots. Return the number of children
* visited. No particular ordering is guaranteed. */
int
-virDomainSnapshotForEachChild(virDomainSnapshotObjListPtr snapshots,
+virDomainSnapshotForEachChild(virDomainSnapshotObjListPtr snapshots
ATTRIBUTE_UNUSED,
virDomainSnapshotObjPtr snapshot,
virHashIterator iter,
void *data)
{
-struct snapshot_act_on_child act;
+virDomainSnapshotObjPtr child = snapshot-first_child;
-act.parent = snapshot-def-name;
-act.number
Re: [libvirt] [PATCH 4/4] snapshot: drop dead parameters
On Fri, Oct 07, 2011 at 06:05:57PM -0600, Eric Blake wrote:
The previous optimizations lead to some follow-on cleanups.
* src/conf/domain_conf.c (virDomainSnapshotForEachChild)
(virDomainSnapshotForEachDescendant): Drop dead parameter.
(virDomainSnapshotActOnDescendant)
(virDomainSnapshotObjListNumFrom)
(virDomainSnapshotObjListGetNamesFrom): Update callers.
* src/qemu/qemu_driver.c (qemuDomainSnapshotNumChildren)
(qemuDomainSnapshotListChildrenNames, qemuDomainSnapshotDelete):
Likewise.
* src/conf/domain_conf.h: Update prototypes.
---
src/conf/domain_conf.c | 20
src/conf/domain_conf.h | 11 ++-
src/qemu/qemu_driver.c | 12 +---
3 files changed, 15 insertions(+), 28 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e77257a..d52a79f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12192,7 +12192,6 @@ cleanup:
}
int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
- virDomainSnapshotObjListPtr
snapshots,
char **const names, int maxnames,
unsigned int flags)
{
@@ -12202,11 +12201,11 @@ int
virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
data.flags = flags ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
if (flags VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
-virDomainSnapshotForEachDescendant(snapshots, snapshot,
+virDomainSnapshotForEachDescendant(snapshot,
virDomainSnapshotObjListCopyNames,
data);
else
-virDomainSnapshotForEachChild(snapshots, snapshot,
+virDomainSnapshotForEachChild(snapshot,
virDomainSnapshotObjListCopyNames,
data);
if (data.oom) {
@@ -12263,7 +12262,6 @@ int
virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
int
virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
-virDomainSnapshotObjListPtr snapshots,
unsigned int flags)
{
struct virDomainSnapshotNumData data = { 0, 0 };
@@ -12271,11 +12269,11 @@
virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
data.flags = flags ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
if (flags VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
-virDomainSnapshotForEachDescendant(snapshots, snapshot,
+virDomainSnapshotForEachDescendant(snapshot,
virDomainSnapshotObjListCount,
data);
else if (data.flags)
-virDomainSnapshotForEachChild(snapshots, snapshot,
+virDomainSnapshotForEachChild(snapshot,
virDomainSnapshotObjListCount, data);
else
data.count = snapshot-nchildren;
@@ -12300,8 +12298,7 @@ void
virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
* other entries in snapshots. Return the number of children
* visited. No particular ordering is guaranteed. */
int
-virDomainSnapshotForEachChild(virDomainSnapshotObjListPtr snapshots
ATTRIBUTE_UNUSED,
- virDomainSnapshotObjPtr snapshot,
+virDomainSnapshotForEachChild(virDomainSnapshotObjPtr snapshot,
virHashIterator iter,
void *data)
{
@@ -12331,15 +12328,14 @@ virDomainSnapshotActOnDescendant(void *payload,
curr-number++;
(curr-iter)(payload, name, curr-data);
-virDomainSnapshotForEachDescendant(NULL, obj, curr-iter, curr-data);
+virDomainSnapshotForEachDescendant(obj, curr-iter, curr-data);
}
/* Run iter(data) on all descendants of snapshot, while ignoring all
* other entries in snapshots. Return the number of descendants
* visited. No particular ordering is guaranteed. */
int
-virDomainSnapshotForEachDescendant(virDomainSnapshotObjListPtr snapshots
ATTRIBUTE_UNUSED,
- virDomainSnapshotObjPtr snapshot,
+virDomainSnapshotForEachDescendant(virDomainSnapshotObjPtr snapshot,
virHashIterator iter,
void *data)
{
@@ -12348,7 +12344,7 @@
virDomainSnapshotForEachDescendant(virDomainSnapshotObjListPtr snapshots
ATTRIBU
act.number = 0;
act.iter = iter;
act.data = data;
-virDomainSnapshotForEachChild(NULL, snapshot,
+virDomainSnapshotForEachChild(snapshot,
virDomainSnapshotActOnDescendant, act);
return act.number;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9b3870a..ce93215 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1464,9
Re: [libvirt] [PATCH 14/7] snapshot: virsh shorthand for operating on current snap
On Thu, Oct 06, 2011 at 05:41:11PM -0600, Eric Blake wrote:
Rather than having to do:
$ virsh snapshot-revert dom $(virsh snapshot-current dom --name)
I thought it would be nice to do:
$ virsh snaphot-revert dom --current
I intentionally made it so that omitting both --current and a name
is an error (having the absence of a name imply --current seems
just a bit too magic, so --current must be explicit).
I didn't add 'virsh snapshot-dumpxml --current' since we already
have 'virsh snapshot-current' for the same task. All other
snapshot-* options that take a snapshotname now take --current in
its place; while keeping snapshot-edit backwards-compatible.
* tools/virsh.c (vshLookupSnapshot): New helper function.
(cmdSnapshotEdit, cmdSnapshotList, cmdSnapshotParent)
(cmdSnapshotDelete, cmdDomainSnapshotRevert): Use it, adding an
option where needed.
* tools/virsh.pod (snapshot-delete, snapshot-edit)
(snapshot-list, snapshot-parent, snapshot-revert): Document
use of --current.
(snapshot-dumpxml): Mention alternative.
---
Does not strictly depend on virDomainSnapshotNumChildren, other
than the fact that I'm touching 'virsh snapshot-list dom --from name'
to add 'virsh snapshot-list dom --current'; I could split that
change out from the rest of the patch if desired to get the ease-of-use
in the other snapshot-* commands before the new API is approved.
tools/virsh.c | 109
---
tools/virsh.pod | 31 ++--
2 files changed, 90 insertions(+), 50 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 86b230d..ea7b56b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12816,6 +12816,48 @@ cleanup:
return ret;
}
+/* Helper for resolving {--current | --ARG name} into a snapshot
+ * belonging to DOM. If EXCLUSIVE, fail if both --current and arg are
+ * present. On success, populate *SNAP, and if NAME is not NULL,
+ * *NAME, before returning 0. On failure, return -1 after issuing an
+ * error message. */
+static int
+vshLookupSnapshot(vshControl *ctl, const vshCmd *cmd,
+ const char *arg, bool exclusive, virDomainPtr dom,
+ virDomainSnapshotPtr *snap, const char **name)
+{
+bool current = vshCommandOptBool(cmd, current);
+const char *snapname = NULL;
+
+if (vshCommandOptString(cmd, arg, snapname) 0) {
+vshError(ctl, _(invalid argument for --%s), arg);
+return -1;
+}
+
+if (exclusive current snapname) {
+vshError(ctl, _(--%s and --current are mutually exclusive), arg);
+return -1;
+}
+
+if (snapname) {
+*snap = virDomainSnapshotLookupByName(dom, snapname, 0);
+} else if (current) {
+*snap = virDomainSnapshotCurrent(dom, 0);
+} else {
+vshError(ctl, _(--%s or --current is required), arg);
+return -1;
+}
+if (!*snap) {
+virshReportError(ctl);
+return -1;
+}
+
+if (name *snap)
+*name = vshStrdup(ctl, virDomainSnapshotGetName(*snap));
+
+return 0;
+}
+
/*
* snapshot-edit command
*/
@@ -12827,7 +12869,7 @@ static const vshCmdInfo info_snapshot_edit[] = {
static const vshCmdOptDef opts_snapshot_edit[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
-{snapshotname, VSH_OT_DATA, VSH_OFLAG_REQ, N_(snapshot name)},
+{snapshotname, VSH_OT_DATA, 0, N_(snapshot name)},
{current, VSH_OT_BOOL, 0, N_(also set edited snapshot as current)},
{NULL, 0, 0, NULL}
};
@@ -12845,21 +12887,19 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE;
unsigned int define_flags = VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
-if (vshCommandOptBool(cmd, current))
+if (vshCommandOptBool(cmd, current)
+vshCommandOptBool(cmd, snapshotname))
define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT;
if (!vshConnectionUsability(ctl, ctl-conn))
return false;
-if (vshCommandOptString(cmd, snapshotname, name) = 0)
-goto cleanup;
-
dom = vshCommandOptDomain(ctl, cmd, NULL);
if (dom == NULL)
goto cleanup;
-snapshot = virDomainSnapshotLookupByName(dom, name, 0);
-if (snapshot == NULL)
+if (vshLookupSnapshot(ctl, cmd, snapshotname, false, dom,
+ snapshot, name) 0)
goto cleanup;
/* Get the XML configuration of the snapshot. */
@@ -13109,6 +13149,8 @@ static const vshCmdOptDef opts_snapshot_list[] = {
N_(list only snapshots that have metadata that would prevent
undefine)},
{tree, VSH_OT_BOOL, 0, N_(list snapshots in a tree)},
{from, VSH_OT_DATA, 0, N_(limit list to children of given snapshot)},
+{current, VSH_OT_BOOL, 0,
+ N_(limit list to children of current snapshot)},
{descendants, VSH_OT_BOOL, 0, N_(with
Re: [libvirt] [PATCHv2 1/7] snapshot: new virDomainSnapshotListChildrenNames API
On Fri, Sep 30, 2011 at 05:09:23PM -0600, Eric Blake wrote:
The previous API addition allowed traversal up the hierarchy;
this one makes it easier to traverse down the hierarchy.
In the python bindings, virDomainSnapshotNumChildren can be
generated, but virDomainSnapshotListChildrenNames had to copy
from the hand-written example of virDomainSnapshotListNames.
* include/libvirt/libvirt.h.in (virDomainSnapshotNumChildren)
(virDomainSnapshotListChildrenNames): New prototypes.
(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS): New flag alias.
* src/libvirt.c (virDomainSnapshotNumChildren)
(virDomainSnapshotListChildrenNames): New functions.
* src/libvirt_public.syms: Export them.
* src/driver.h (virDrvDomainSnapshotNumChildren)
(virDrvDomainSnapshotListChildrenNames): New callbacks.
* python/generator.py (skip_impl, nameFixup): Update lists.
* python/libvirt-override-api.xml: Likewise.
* python/libvirt-override.c
(libvirt_virDomainSnapshotListChildrenNames): New wrapper function.
---
v2: no change
include/libvirt/libvirt.h.in| 27 +++--
python/generator.py |4 ++
python/libvirt-override-api.xml | 12 +++-
python/libvirt-override.c | 45
src/driver.h| 12
src/libvirt.c | 111
+++
src/libvirt_public.syms |2 +
7 files changed, 204 insertions(+), 9 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a832b65..7403a9a 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2686,13 +2686,19 @@ virDomainSnapshotPtr
virDomainSnapshotCreateXML(virDomainPtr domain,
char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
unsigned int flags);
-/* Flags valid for both virDomainSnapshotNum() and
- * virDomainSnapshotListNames(). */
+/* Flags valid for virDomainSnapshotNum(),
+ * virDomainSnapshotListNames(), virDomainSnapshotNumChildren(), and
+ * virDomainSnapshotListChildrenNames(). Note that the interpretation
+ * of flag (10) depends on which function it is passed to. */
typedef enum {
-VIR_DOMAIN_SNAPSHOT_LIST_ROOTS= (1 0), /* Filter by snapshots
which
- have no parents */
-VIR_DOMAIN_SNAPSHOT_LIST_METADATA = (1 1), /* Filter by snapshots
which
- have metadata */
+VIR_DOMAIN_SNAPSHOT_LIST_ROOTS = (1 0), /* Filter by snapshots
+with no parents, when
+listing a domain */
+VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS = (1 0), /* List all descendants,
+not just children,
when
+listing a snapshot */
+VIR_DOMAIN_SNAPSHOT_LIST_METADATA= (1 1), /* Filter by snapshots
+which have metadata
*/
} virDomainSnapshotListFlags;
Hum, okay, a tad bit confusing though
/* Return the number of snapshots for this domain */
@@ -2702,6 +2708,15 @@ int virDomainSnapshotNum(virDomainPtr domain, unsigned
int flags);
int virDomainSnapshotListNames(virDomainPtr domain, char **names, int
nameslen,
unsigned int flags);
+/* Return the number of child snapshots for this snapshot */
+int virDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot,
+ unsigned int flags);
+
+/* Get the names of all child snapshots for this snapshot */
+int virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
+ char **names, int nameslen,
+ unsigned int flags);
+
/* Get a handle to a named snapshot */
virDomainSnapshotPtr virDomainSnapshotLookupByName(virDomainPtr domain,
const char *name,
Okay
diff --git a/python/generator.py b/python/generator.py
index 79558dd..71afdb7 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -352,6 +352,7 @@ skip_impl = (
'virConnectListDefinedInterfaces',
'virConnectListNWFilters',
'virDomainSnapshotListNames',
+'virDomainSnapshotListChildrenNames',
'virConnGetLastError',
'virGetLastError',
'virDomainGetInfo',
@@ -963,6 +964,9 @@ def nameFixup(name, classe, type, file):
elif name[0:26] == virDomainSnapshotListNames:
func = name[9:]
func = string.lower(func[0:1]) + func[1:]
+elif name[0:28] == virDomainSnapshotNumChildren:
+func = name[17:]
+func = string.lower(func[0:1]) + func[1:]
elif name[0:20] == virDomainSnapshotNum:
func = name[9:]
func =
Re: [libvirt] [PATCHv2 2/7] snapshot: virsh snapshot-list and children
On Fri, Sep 30, 2011 at 05:09:24PM -0600, Eric Blake wrote:
Sometimes, we only care about one branch of the snapshot hierarchy.
Make it easier to list a single branch, by using the new APIs.
Technically, I could emulate these new virsh options on old servers
by doing a complete dump, then scraping xml to filter out just the
snapshots that I care about, but I didn't want to do that in this patch.
* tools/virsh.c (cmdSnapshotList): Add --from, --descendants.
* tools/virsh.pod (snapshot-list): Document them.
---
v2: minor rebase cleanups
tools/virsh.c | 76
---
tools/virsh.pod |9 ++-
2 files changed, 69 insertions(+), 16 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 955b8df..adafe86 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13102,6 +13102,8 @@ static const vshCmdOptDef opts_snapshot_list[] = {
{metadata, VSH_OT_BOOL, 0,
N_(list only snapshots that have metadata that would prevent
undefine)},
{tree, VSH_OT_BOOL, 0, N_(list snapshots in a tree)},
+{from, VSH_OT_DATA, 0, N_(limit list to children of given snapshot)},
+{descendants, VSH_OT_BOOL, 0, N_(with --from, list all descendants)},
{NULL, 0, 0, NULL}
};
@@ -13128,25 +13130,36 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
char timestr[100];
struct tm time_info;
bool tree = vshCommandOptBool(cmd, tree);
+const char *from = NULL;
+virDomainSnapshotPtr start = NULL;
+
+if (vshCommandOptString(cmd, from, from) 0) {
+vshError(ctl, _(invalid from argument '%s'), from);
+goto cleanup;
+}
if (vshCommandOptBool(cmd, parent)) {
if (vshCommandOptBool(cmd, roots)) {
vshError(ctl, %s,
- _(--parent and --roots are mutually exlusive));
+ _(--parent and --roots are mutually exclusive));
return false;
}
if (tree) {
vshError(ctl, %s,
- _(--parent and --tree are mutually exlusive));
+ _(--parent and --tree are mutually exclusive));
return false;
}
parent_filter = 1;
} else if (vshCommandOptBool(cmd, roots)) {
if (tree) {
vshError(ctl, %s,
- _(--roots and --tree are mutually exlusive));
+ _(--roots and --tree are mutually exclusive));
return false;
}
+if (from) {
+vshError(ctl, %s,
+ _(--roots and --from are mutually exclusive));
+}
flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
}
@@ -13161,16 +13174,29 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (dom == NULL)
goto cleanup;
-numsnaps = virDomainSnapshotNum(dom, flags);
-
-/* Fall back to simulation if --roots was unsupported. */
-if (numsnaps 0 last_error-code == VIR_ERR_INVALID_ARG
-(flags VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
-virFreeError(last_error);
-last_error = NULL;
-parent_filter = -1;
-flags = ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+if (from) {
+start = virDomainSnapshotLookupByName(dom, from, 0);
+if (!start)
+goto cleanup;
+if (vshCommandOptBool(cmd, descendants) || tree) {
+flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+}
+numsnaps = virDomainSnapshotNumChildren(start, flags);
+if (numsnaps = 0 tree)
+numsnaps++;
+/* XXX Is it worth emulating --from on older servers? */
+} else {
numsnaps = virDomainSnapshotNum(dom, flags);
+
+/* Fall back to simulation if --roots was unsupported. */
+if (numsnaps 0 last_error-code == VIR_ERR_INVALID_ARG
+(flags VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
+virFreeError(last_error);
+last_error = NULL;
+parent_filter = -1;
+flags = ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+numsnaps = virDomainSnapshotNum(dom, flags);
+}
}
if (numsnaps 0)
@@ -13196,14 +13222,32 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (VIR_ALLOC_N(names, numsnaps) 0)
goto cleanup;
-actual = virDomainSnapshotListNames(dom, names, numsnaps, flags);
+if (from) {
+/* When mixing --from and --tree, we want to start the tree at the
+ * given snapshot. Without --tree, only list the children. */
+if (tree) {
+if (numsnaps)
+actual = virDomainSnapshotListChildrenNames(start, names + 1,
+numsnaps - 1,
+flags);
+if (actual = 0) {
+actual++;
+names[0] =
Re: [libvirt] [PATCHv2 3/7] snapshot: virsh fallback for snapshot-list --tree --from
On Fri, Sep 30, 2011 at 05:09:25PM -0600, Eric Blake wrote:
Emulating --from requires grabbing the entire list of snapshots
and their parents, and recursively iterating over the list from
the point of interest - but we already do that for --tree. This
turns on emulation for that situation.
* tools/virsh.c (__vshControl): Rename member.
(vshReconnect, cmdConnect, vshGetSnapshotParent): Update clients.
(cmdSnapshotList): Add fallback.
---
v2: reuse existing ctl flag, so that virsh in shell mode doesn't
repeatedly try non-working commands
tools/virsh.c | 43 +--
1 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index adafe86..e3bc364 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -246,8 +246,8 @@ typedef struct __vshControl {
char *historyfile; /* readline history file name */
bool useGetInfo;/* must use virDomainGetInfo, since
virDomainGetState is not supported */
-bool useSnapshotGetXML; /* must use virDomainSnapshotGetXMLDesc,
since
- virDomainSnapshotGetParent is missing */
+bool useSnapshotOld;/* cannot use virDomainSnapshotGetParent or
+ virDomainSnapshotNumChildren */
} __vshControl;
typedef struct vshCmdGrp {
@@ -599,7 +599,7 @@ vshReconnect(vshControl *ctl)
vshError(ctl, %s, _(Reconnected to the hypervisor));
disconnected = 0;
ctl-useGetInfo = false;
-ctl-useSnapshotGetXML = false;
+ctl-useSnapshotOld = false;
}
/* ---
@@ -747,7 +747,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
ctl-name = vshStrdup(ctl, name);
ctl-useGetInfo = false;
-ctl-useSnapshotGetXML = false;
+ctl-useSnapshotOld = false;
ctl-readonly = ro;
ctl-conn = virConnectOpenAuth(ctl-name, virConnectAuthPtrDefault,
@@ -13042,7 +13042,7 @@ vshGetSnapshotParent(vshControl *ctl,
virDomainSnapshotPtr snapshot,
*parent_name = NULL;
/* Try new API, since it is faster. */
-if (!ctl-useSnapshotGetXML) {
+if (!ctl-useSnapshotOld) {
parent = virDomainSnapshotGetParent(snapshot, 0);
if (parent) {
/* API works, and virDomainSnapshotGetName will be accurate */
@@ -13056,7 +13056,7 @@ vshGetSnapshotParent(vshControl *ctl,
virDomainSnapshotPtr snapshot,
goto cleanup;
}
/* API didn't work, fall back to XML scraping. */
-ctl-useSnapshotGetXML = true;
+ctl-useSnapshotOld = true;
}
xml = virDomainSnapshotGetXMLDesc(snapshot, 0);
@@ -13181,10 +13181,22 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, descendants) || tree) {
flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
}
-numsnaps = virDomainSnapshotNumChildren(start, flags);
-if (numsnaps = 0 tree)
-numsnaps++;
-/* XXX Is it worth emulating --from on older servers? */
+numsnaps = ctl-useSnapshotOld ? -1 :
+virDomainSnapshotNumChildren(start, flags);
+/* XXX Is it worth emulating --from without --tree on older servers?
*/
+if (tree) {
+if (numsnaps = 0) {
+numsnaps++;
+} else if (ctl-useSnapshotOld ||
+ last_error-code == VIR_ERR_NO_SUPPORT) {
+/* We can emulate --tree --from. */
+virFreeError(last_error);
+last_error = NULL;
+ctl-useSnapshotOld = true;
+flags = ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+numsnaps = virDomainSnapshotNum(dom, flags);
+}
+}
} else {
numsnaps = virDomainSnapshotNum(dom, flags);
@@ -13199,8 +13211,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
}
}
-if (numsnaps 0)
+if (numsnaps 0) {
+if (!last_error)
+vshError(ctl, _(missing support));
goto cleanup;
+}
if (!tree) {
if (parent_filter 0)
@@ -13222,7 +13237,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (VIR_ALLOC_N(names, numsnaps) 0)
goto cleanup;
-if (from) {
+if (from !ctl-useSnapshotOld) {
/* When mixing --from and --tree, we want to start the tree at the
* given snapshot. Without --tree, only list the children. */
if (tree) {
@@ -13247,7 +13262,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (tree) {
char indentBuf[INDENT_BUFLEN];
char **parents = vshCalloc(ctl, sizeof(char *), actual);
-for (i = (from ? 1 : 0); i actual; i++) {
+for (i = (from !ctl-useSnapshotOld); i actual; i++) {
/* free up memory from previous iterations
Re: [libvirt] [PATCHv2 4/7] snapshot: virsh fallback for snapshot-list --from children
On Fri, Sep 30, 2011 at 05:09:26PM -0600, Eric Blake wrote:
Iterating over one level of children requires parsing all snapshots
and their parents; a bit of code shuffling makes it pretty easy
to do this as well.
* tools/virsh.c (cmdSnapshotList): Add another fallback.
---
v2: fix compilation error, rebase around ctl flag
tools/virsh.c | 48
1 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index e3bc364..b2523d3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13117,6 +13117,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
information needed, 1 for parent column */
int numsnaps;
char **names = NULL;
+char **parents = NULL;
int actual = 0;
int i;
xmlDocPtr xml = NULL;
@@ -13132,6 +13133,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
bool tree = vshCommandOptBool(cmd, tree);
const char *from = NULL;
virDomainSnapshotPtr start = NULL;
+bool descendants = false;
if (vshCommandOptString(cmd, from, from) 0) {
vshError(ctl, _(invalid from argument '%s'), from);
@@ -13175,27 +13177,29 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
if (from) {
+descendants = vshCommandOptBool(cmd, descendants);
start = virDomainSnapshotLookupByName(dom, from, 0);
if (!start)
goto cleanup;
-if (vshCommandOptBool(cmd, descendants) || tree) {
+if (descendants || tree) {
flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
}
numsnaps = ctl-useSnapshotOld ? -1 :
virDomainSnapshotNumChildren(start, flags);
-/* XXX Is it worth emulating --from without --tree on older servers?
*/
-if (tree) {
-if (numsnaps = 0) {
-numsnaps++;
-} else if (ctl-useSnapshotOld ||
- last_error-code == VIR_ERR_NO_SUPPORT) {
-/* We can emulate --tree --from. */
+if (numsnaps 0) {
+/* XXX also want to emulate --descendants without --tree */
+if ((!descendants || tree)
+(ctl-useSnapshotOld ||
+ last_error-code == VIR_ERR_NO_SUPPORT)) {
+/* We can emulate --from. */
virFreeError(last_error);
last_error = NULL;
ctl-useSnapshotOld = true;
flags = ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
numsnaps = virDomainSnapshotNum(dom, flags);
}
+} else if (tree) {
+numsnaps++;
}
} else {
numsnaps = virDomainSnapshotNum(dom, flags);
@@ -13259,9 +13263,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (actual 0)
goto cleanup;
-if (tree) {
-char indentBuf[INDENT_BUFLEN];
-char **parents = vshCalloc(ctl, sizeof(char *), actual);
+if (!tree)
+qsort(names[0], actual, sizeof(char*), namesorter);
+
+if (tree || ctl-useSnapshotOld) {
+parents = vshCalloc(ctl, sizeof(char *), actual);
for (i = (from !ctl-useSnapshotOld); i actual; i++) {
/* free up memory from previous iterations of the loop */
if (snapshot)
@@ -13275,6 +13281,9 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
}
+}
+if (tree) {
+char indentBuf[INDENT_BUFLEN];
for (i = 0 ; i actual ; i++) {
memset(indentBuf, '\0', sizeof indentBuf);
if (ctl-useSnapshotOld ? STREQ(names[i], from) : !parents[i])
@@ -13288,16 +13297,19 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
0,
indentBuf);
}
-for (i = 0 ; i actual ; i++)
-VIR_FREE(parents[i]);
-VIR_FREE(parents);
ret = true;
goto cleanup;
} else {
-qsort(names[0], actual, sizeof(char*), namesorter);
+if (ctl-useSnapshotOld descendants) {
+/* XXX emulate --descendants as well */
+goto cleanup;
+}
for (i = 0; i actual; i++) {
+if (ctl-useSnapshotOld STRNEQ_NULLABLE(parents[i], from))
+continue;
+
/* free up memory from previous iterations of the loop */
VIR_FREE(parent);
VIR_FREE(state);
@@ -13362,9 +13374,13 @@ cleanup:
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
VIR_FREE(doc);
-for (i = 0; i actual; i++)
+for (i = 0; i actual; i++) {
VIR_FREE(names[i]);
+if (parents)
+VIR_FREE(parents[i]);
+}
VIR_FREE(names);
+VIR_FREE(parents);
if (dom)
Re: [libvirt] [PATCHv2 5/7] snapshot: virsh fallback for snapshot-list --descendants --from
On Fri, Sep 30, 2011 at 05:09:27PM -0600, Eric Blake wrote:
Given a list of snapshots and their parents, finding all descendants
requires a hairy traversal. This code is O(n^3); it could maybe be
made to scale O(n^2) with the use of a hash table, but that costs more
memory. Hopefully there aren't too many people with a hierarchy
so large as to approach REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX (1024).
* tools/virsh.c (cmdSnapshotList): Add final fallback.
---
v2: rebase around ctl flag
tools/virsh.c | 67
+++--
1 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index b2523d3..ec6f854 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13133,6 +13133,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
bool tree = vshCommandOptBool(cmd, tree);
const char *from = NULL;
virDomainSnapshotPtr start = NULL;
+int start_index = -1;
bool descendants = false;
if (vshCommandOptString(cmd, from, from) 0) {
@@ -13187,10 +13188,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
numsnaps = ctl-useSnapshotOld ? -1 :
virDomainSnapshotNumChildren(start, flags);
if (numsnaps 0) {
-/* XXX also want to emulate --descendants without --tree */
-if ((!descendants || tree)
-(ctl-useSnapshotOld ||
- last_error-code == VIR_ERR_NO_SUPPORT)) {
+if (ctl-useSnapshotOld ||
+last_error-code == VIR_ERR_NO_SUPPORT) {
/* We can emulate --from. */
virFreeError(last_error);
last_error = NULL;
@@ -13269,6 +13268,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (tree || ctl-useSnapshotOld) {
parents = vshCalloc(ctl, sizeof(char *), actual);
for (i = (from !ctl-useSnapshotOld); i actual; i++) {
+if (ctl-useSnapshotOld STREQ(names[i], from)) {
+start_index = i;
+continue;
+}
+
/* free up memory from previous iterations of the loop */
if (snapshot)
virDomainSnapshotFree(snapshot);
@@ -13302,12 +13306,61 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
} else {
if (ctl-useSnapshotOld descendants) {
-/* XXX emulate --descendants as well */
-goto cleanup;
+bool changed = false;
+
+/* Make multiple passes over the list - first pass NULLs
+ * out all roots except start, remaining passes NULL out
+ * any entry whose parent is not still in list. Also, we
+ * NULL out parent when name is known to be in list.
+ * Sorry, this is O(n^3) - hope your hierarchy isn't huge. */
+if (start_index 0) {
+vshError(ctl, _(snapshot %s disappeared from list), from);
+goto cleanup;
+}
+for (i = 0; i actual; i++) {
+if (i == start_index)
+continue;
+if (!parents[i]) {
+VIR_FREE(names[i]);
+} else if (STREQ(parents[i], from)) {
+VIR_FREE(parents[i]);
+changed = true;
+}
+}
+if (!changed) {
+ret = true;
+goto cleanup;
+}
+while (changed) {
+changed = false;
+for (i = 0; i actual; i++) {
+bool found = false;
+int j;
+
+if (!names[i] || !parents[i])
+continue;
+for (j = 0; j actual; j++) {
+if (!names[j] || i == j)
+continue;
+if (STREQ(parents[i], names[j])) {
+found = true;
+if (!parents[j])
+VIR_FREE(parents[i]);
+break;
+}
+}
+if (!found) {
+changed = true;
+VIR_FREE(names[i]);
+}
+}
+}
+VIR_FREE(names[start_index]);
}
for (i = 0; i actual; i++) {
-if (ctl-useSnapshotOld STRNEQ_NULLABLE(parents[i], from))
+if (ctl-useSnapshotOld
+(descendants ? !names[i] : STRNEQ_NULLABLE(parents[i],
from)))
continue;
/* free up memory from previous iterations of the loop */
ACK, to be removed in second patch set anyway :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
Re: [libvirt] [PATCHv2 6/7] snapshot: remote protocol for snapshot children
On Fri, Sep 30, 2011 at 05:09:28PM -0600, Eric Blake wrote:
Very mechanical. I'm so glad we've automated the generation of things,
compared to what it was in 0.8.x days, where this would be much longer.
* src/remote/remote_protocol.x
(REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN)
(REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES): New rpcs.
(remote_domain_snapshot_num_children_args)
(remote_domain_snapshot_num_children_ret)
(remote_domain_snapshot_list_children_names_args)
(remote_domain_snapshot_list_children_names_ret): New structs.
* src/remote/remote_driver.c (remote_driver): Use it.
* src/remote_protocol-structs: Update.
---
v2: fix typo in remote_protocol-structs
src/remote/remote_driver.c |2 ++
src/remote/remote_protocol.x | 25 +++--
src/remote_protocol-structs | 20
3 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 83f4f3c..0e303df 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -4411,6 +4411,8 @@ static virDriver remote_driver = {
.domainSnapshotGetXMLDesc = remoteDomainSnapshotGetXMLDesc, /* 0.8.0 */
.domainSnapshotNum = remoteDomainSnapshotNum, /* 0.8.0 */
.domainSnapshotListNames = remoteDomainSnapshotListNames, /* 0.8.0 */
+.domainSnapshotNumChildren = remoteDomainSnapshotNumChildren, /* 0.9.7 */
+.domainSnapshotListChildrenNames =
remoteDomainSnapshotListChildrenNames, /* 0.9.7 */
.domainSnapshotLookupByName = remoteDomainSnapshotLookupByName, /* 0.8.0
*/
.domainHasCurrentSnapshot = remoteDomainHasCurrentSnapshot, /* 0.8.0 */
.domainSnapshotGetParent = remoteDomainSnapshotGetParent, /* 0.9.7 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index c8a92fd..f95253e 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2067,6 +2067,25 @@ struct remote_domain_snapshot_list_names_ret {
remote_nonnull_string namesREMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX; /*
insert@1 */
};
+struct remote_domain_snapshot_num_children_args {
+remote_nonnull_domain_snapshot snap;
+unsigned int flags;
+};
+
+struct remote_domain_snapshot_num_children_ret {
+int num;
+};
+
+struct remote_domain_snapshot_list_children_names_args {
+remote_nonnull_domain_snapshot snap;
+int maxnames;
+unsigned int flags;
+};
+
+struct remote_domain_snapshot_list_children_names_ret {
+remote_nonnull_string namesREMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX; /*
insert@1 */
+};
+
struct remote_domain_snapshot_lookup_by_name_args {
remote_nonnull_domain dom;
remote_nonnull_string name;
@@ -2524,8 +2543,10 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, /* autogen autogen */
REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, /* skipgen skipgen */
-REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen */
-REMOTE_PROC_DOMAIN_RESET = 245 /* autogen autogen */
+REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen
priority:high */
+REMOTE_PROC_DOMAIN_RESET = 245, /* autogen autogen */
+REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, /* autogen autogen
priority:high */
+REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247 /* autogen autogen
priority:high */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 69175cc..7894441 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -1557,6 +1557,24 @@ struct remote_domain_snapshot_list_names_ret {
remote_nonnull_string * names_val;
} names;
};
+struct remote_domain_snapshot_num_children_args {
+remote_nonnull_domain_snapshot snap;
+u_int flags;
+};
+struct remote_domain_snapshot_num_children_ret {
+intnum;
+};
+struct remote_domain_snapshot_list_children_names_args {
+remote_nonnull_domain_snapshot snap;
+intmaxnames;
+u_int flags;
+};
+struct remote_domain_snapshot_list_children_names_ret {
+struct {
+u_int names_len;
+remote_nonnull_string * names_val;
+} names;
+};
struct remote_domain_snapshot_lookup_by_name_args {
remote_nonnull_domain dom;
remote_nonnull_string name;
@@ -1971,4 +1989,6 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243,
REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244,
REMOTE_PROC_DOMAIN_RESET = 245,
+REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246,
+REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247,
};
Re: [libvirt] [PATCHv2 7/7] snapshot: implement snapshot children listing in qemu
On Fri, Sep 30, 2011 at 05:09:29PM -0600, Eric Blake wrote:
Not too hard to wire up. The trickiest part is realizing that
listing children of a snapshot cannot use SNAPSHOT_LIST_ROOTS,
and that we overloaded that bit to also mean SNAPSHOT_LIST_DESCENDANTS;
we use that bit to decide which iteration to use, but don't want
the existing counting/listing functions to see that bit.
* src/conf/domain_conf.h (virDomainSnapshotObjListNumFrom)
(virDomainSnapshotObjListGetNamesFrom): New prototypes.
* src/conf/domain_conf.c (virDomainSnapshotObjListNumFrom)
(virDomainSnapshotObjListGetNamesFrom): New functions.
* src/libvirt_private.syms (domain_conf.h): Export them.
* src/qemu/qemu_driver.c (qemuDomainSnapshotNumChildren)
(qemuDomainSnapshotListChildrenNames): New functions.
---
v2: no change, but now virsh changes have been tested both with
and without this patch
src/conf/domain_conf.c | 51 +++
src/conf/domain_conf.h |7
src/libvirt_private.syms |2 +
src/qemu/qemu_driver.c | 87
++
4 files changed, 147 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c141982..438e3b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12138,6 +12138,37 @@ cleanup:
return -1;
}
+int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
+ virDomainSnapshotObjListPtr
snapshots,
+ char **const names, int maxnames,
+ unsigned int flags)
+{
+struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, 0 };
+int i;
+
+data.flags = flags ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+
+if (flags VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
+virDomainSnapshotForEachDescendant(snapshots, snapshot,
+ virDomainSnapshotObjListCopyNames,
+ data);
+else
+virDomainSnapshotForEachChild(snapshots, snapshot,
+ virDomainSnapshotObjListCopyNames,
data);
+
+if (data.oom) {
+virReportOOMError();
+goto cleanup;
+}
+
+return data.numnames;
+
+cleanup:
+for (i = 0; i data.numnames; i++)
+VIR_FREE(data.names[i]);
+return -1;
+}
+
struct virDomainSnapshotNumData {
int count;
unsigned int flags;
@@ -12165,6 +12196,26 @@ int
virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
return data.count;
}
+int
+virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
+virDomainSnapshotObjListPtr snapshots,
+unsigned int flags)
+{
+struct virDomainSnapshotNumData data = { 0, 0 };
+
+data.flags = flags ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+
+if (flags VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
+virDomainSnapshotForEachDescendant(snapshots, snapshot,
+ virDomainSnapshotObjListCount,
+ data);
+else
+virDomainSnapshotForEachChild(snapshots, snapshot,
+ virDomainSnapshotObjListCount, data);
+
+return data.count;
+}
+
virDomainSnapshotObjPtr
virDomainSnapshotFindByName(const virDomainSnapshotObjListPtr snapshots,
const char *name)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0bc0042..1258740 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1488,6 +1488,13 @@ int
virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
unsigned int flags);
int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
unsigned int flags);
+int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
+ virDomainSnapshotObjListPtr
snapshots,
+ char **const names, int maxnames,
+ unsigned int flags);
+int virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
+virDomainSnapshotObjListPtr snapshots,
+unsigned int flags);
virDomainSnapshotObjPtr virDomainSnapshotFindByName(const
virDomainSnapshotObjListPtr snapshots,
const char *name);
void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c2a3fab..6b9ceaa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -413,7 +413,9 @@
[libvirt] [v2] storage: Do not use comma as seperator for lvs output
* src/storage/storage_backend_logical.c:
If a logical vol is created as striped. (e.g. --stripes 3),
the device field of lvs output will have multiple fileds which are
seperated by comma. Thus the RE we write in the codes will not
work well anymore. E.g. (lvs output for a stripped vol, uses # as
seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\
/dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304
The RE we use:
const char *regexes[] = {
^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$
};
Also the RE doesn't match the devices field of striped vol properly,
it contains multiple device path and offset.
This patch mainly does:
1) Change the seperator into #
2) Change the RE for devices field from (\\S+)\\((\\S+)\\)
into (\\S+).
3) Add two new options for lvs command, (segtype, stripes)
4) Extend the RE to match the value for the two new fields.
5) Parse the devices field seperately in virStorageBackendLogicalMakeVol,
multiple extents info are generated if the vol is striped. The
number of extents is equal to the stripes number of the striped vol.
A incidental fix: (virStorageBackendLogicalMakeVol)
Free vol if it's new created and there is error.
Demo on striped vol with the patch applied:
% virsh vol-dumpxml /dev/test_vg/vol_striped2
volume
namevol_striped2/name
keyQuWqmn-kIkZ-IATt-67rc-OWEP-1PHX-Cl2ICs/key
source
device path='/dev/sda5'
extent start='79691776' end='88080384'/
/device
device path='/dev/sda6'
extent start='62914560' end='71303168'/
/device
/source
capacity8388608/capacity
allocation8388608/allocation
target
path/dev/test_vg/vol_striped2/path
permissions
mode0660/mode
owner0/owner
group6/group
labelsystem_u:object_r:fixed_disk_device_t:s0/label
/permissions
/target
/volume
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474
v1 - v2:
v1 just simply changes the seperator into #.
---
src/storage/storage_backend_logical.c | 156 +
1 files changed, 121 insertions(+), 35 deletions(-)
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 589f486..dda68fd 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -62,13 +62,22 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
}
+#define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED striped
+
static int
virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
char **const groups,
void *data)
{
virStorageVolDefPtr vol = NULL;
+bool is_new_vol = false;
unsigned long long offset, size, length;
+const char *regex_unit = (\\S+)\\((\\S+)\\);
+char *regex = NULL;
+regex_t *reg = NULL;
+regmatch_t *vars = NULL;
+char *p = NULL;
+int i, err, nextents, nvars, ret = -1;
/* See if we're only looking for a specific volume */
if (data != NULL) {
@@ -88,19 +97,18 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
return -1;
}
+is_new_vol = true;
vol-type = VIR_STORAGE_VOL_BLOCK;
if ((vol-name = strdup(groups[0])) == NULL) {
virReportOOMError();
-virStorageVolDefFree(vol);
-return -1;
+goto cleanup;
}
if (VIR_REALLOC_N(pool-volumes.objs,
pool-volumes.count + 1)) {
virReportOOMError();
-virStorageVolDefFree(vol);
-return -1;
+goto cleanup;
}
pool-volumes.objs[pool-volumes.count++] = vol;
}
@@ -109,8 +117,7 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
if (virAsprintf(vol-target.path, %s/%s,
pool-def-target.path, vol-name) 0) {
virReportOOMError();
-virStorageVolDefFree(vol);
-return -1;
+goto cleanup;
}
}
@@ -118,8 +125,7 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
if (virAsprintf(vol-backingStore.path, %s/%s,
pool-def-target.path, groups[1]) 0) {
virReportOOMError();
-virStorageVolDefFree(vol);
-return -1;
+goto cleanup;
}
vol-backingStore.format = VIR_STORAGE_POOL_LOGICAL_LVM2;
@@ -128,47 +134,127 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr
pool,
if (vol-key == NULL
(vol-key = strdup(groups[2])) == NULL) {
virReportOOMError();
-return -1;
+goto cleanup;
}
if (virStorageBackendUpdateVolInfo(vol, 1) 0)
-return -1;
+goto cleanup;
+nextents = 1;
+if (STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED)) {
+if (virStrToLong_i(groups[5], NULL, 10, nextents) 0) {
+
Re: [libvirt] [PATCH] qemu: Do not reattach PCI device used by other domain when shutdown
Anybody could help review this? Thanks
Osier
于 2011年09月27日 14:53, Osier Yang 写道:
When failing on starting a domain, it tries to reattach all the PCI
devices defined in the domain conf, regardless of whether the devices
are still used by other domain. This will cause the devices are deleted
from the list qemu_driver-activePciHostdevs, thus the devices will be
thought as usable even if it's not true. And following commands
nodedev-{reattach,reset} will be successful.
How to reproduce:
1) Define two domains with same PCI device defined in the confs.
2) # virsh start domain1
3) # virsh start domain2
4) # virsh nodedev-reattach $pci_device
You will see the device will be reattached to host successfully.
As pciDeviceReattach just check if the device is still used by
other domain via checking if the device is in list driver-activePciHostdevs,
however, the device is deleted from the list by step 2).
This patch is to prohibit the bug by:
1) Prohibit a domain starting or device attachment right at
preparation period (qemuPrepareHostdevPCIDevices) if the
device is in list driver-activePciHostdevs, which means
it's used by other domain.
2) Introduces a new field for struct _pciDevice, (char *used_by),
it will be set as the domain name at preparation period,
(qemuPrepareHostdevPCIDevices). Thus we can prohibit deleting
the device from driver-activePciHostdevs if it's still used by
other domain when stopping the domain process.
* src/pci.h (define two internal functions, pciDeviceSetUsedBy and
pciDevceGetUsedBy)
* src/pci.c (new field char *used_by for struct _pciDevice,
implementations for the two new functions)
* src/libvirt_private.syms (Add the two new internal functions)
* src/qemu_hostdev.h (Modify the definition of functions
qemuPrepareHostdevPCIDevices, and qemuDomainReAttachHostdevDevices)
* src/qemu_hostdev.c (Prohibit preparation and don't delete the
device from activePciHostdevs list if it's still used by other domain)
* src/qemu_hotplug.c (Update function usage, as the definitions are
changed)
---
src/libvirt_private.syms |2 ++
src/qemu/qemu_hostdev.c | 31 ---
src/qemu/qemu_hostdev.h |2 ++
src/qemu/qemu_hotplug.c |4 ++--
src/util/pci.c | 22 ++
src/util/pci.h |3 +++
6 files changed, 59 insertions(+), 5 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8235ea1..a5c5e6c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -872,6 +872,7 @@ virNWFilterHashTableRemoveEntry;
pciDettachDevice;
pciDeviceFileIterate;
pciDeviceGetManaged;
+pciDeviceGetUsedBy;
pciDeviceIsAssignable;
pciDeviceIsVirtualFunction;
pciDeviceListAdd;
@@ -884,6 +885,7 @@ pciDeviceListSteal;
pciDeviceNetName;
pciDeviceReAttachInit;
pciDeviceSetManaged;
+pciDeviceSetUsedBy;
pciFreeDevice;
pciGetDevice;
pciGetPhysicalFunction;
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 6f77717..ef9e3b7 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -101,6 +101,7 @@ cleanup:
int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
+ const char *name,
virDomainHostdevDefPtr *hostdevs,
int nhostdevs)
{
@@ -126,7 +127,10 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver
*driver,
for (i = 0; i pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i);
if (!pciDeviceIsAssignable(dev, !driver-relaxedACS))
-goto reattachdevs;
+goto cleanup;
+
+if (pciDeviceListFind(driver-activePciHostdevs, dev))
+goto cleanup;
if (pciDeviceGetManaged(dev)
pciDettachDevice(dev, driver-activePciHostdevs) 0)
@@ -156,6 +160,14 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver
*driver,
pciDeviceListSteal(pcidevs, dev);
}
+/* Now set the used_by_domain of the device in driver-activePciHostdevs
+ * as domain name.
+ */
+for (i = 0; i pciDeviceListCount(driver-activePciHostdevs); i++) {
+pciDevice * dev = pciDeviceListGet(driver-activePciHostdevs, i);
+pciDeviceSetUsedBy(dev, name);
+}
+
ret = 0;
goto cleanup;
@@ -183,7 +195,7 @@ static int
qemuPrepareHostPCIDevices(struct qemud_driver *driver,
virDomainDefPtr def)
{
-return qemuPrepareHostdevPCIDevices(driver, def-hostdevs,
def-nhostdevs);
+return qemuPrepareHostdevPCIDevices(driver, def-name, def-hostdevs,
def-nhostdevs);
}
@@ -258,11 +270,13 @@ void qemuReattachPciDevice(pciDevice *dev, struct
qemud_driver *driver)
void qemuDomainReAttachHostdevDevices(struct qemud_driver *driver,
+
