- Devel - libvirt List Archives

[libvirt] [PATCH] util: fix index when building lock owners array
by Laine Stump 14 Nov '12

14 Nov '12

The "restart" function for locks allocates a new array according to and pre-sets its length, then reads the owner pids from a JSON document in a loop. Rather than adding each owner at a different index, though, it repeatedly overwrites the last element of the array with all the owners. --- src/util/virlockspace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 7e8c0a7..99b6182 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -435,7 +435,7 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object) goto error; } - res->owners[res->nOwners-1] = (pid_t)owner; + res->owners[j] = (pid_t)owner; } if (virHashAddEntry(lockspace->resources, res->name, res) < 0) { -- 1.7.11.7

2 2

[libvirt] [PATCH] Fix uninitialized variable in virLXCControllerSetupDevPTS
by Daniel P. Berrange 14 Nov '12

14 Nov '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> The lack of initialization of 'opts' caused a SEGV in the cleanup: path if the root->src directory did not exist Pushed under trivial rule --- src/lxc/lxc_controller.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index a41c903..2f1e8a5 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1144,7 +1144,7 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) { virDomainFSDefPtr root = virDomainGetRootFilesystem(ctrl->def); char *mount_options = NULL; - char *opts; + char *opts = NULL; char *devpts = NULL; int ret = -1; -- 1.7.11.7

1 0

[libvirt] [libvirt-glib v2] gobject: Fix GMutex leak
by Christophe Fergeau 14 Nov '12

14 Nov '12

When GLib deprecated g_mutex_new/g_mutex_free, we introduced a compatibility wrapper to implement these using non-deprecated functions. This was done by allocating 0-filled memory by the mutex, and then letting GLib initialize the structure when needed. However, we must call g_mutex_clear when destroying the mutex to free these resources, which this patch fix. --- libvirt-gobject/libvirt-gobject-compat.c | 16 ++++++++++++++++ libvirt-gobject/libvirt-gobject-compat.h | 8 ++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-compat.c b/libvirt-gobject/libvirt-gobject-compat.c index 99cd44c..14b5eb3 100644 --- a/libvirt-gobject/libvirt-gobject-compat.c +++ b/libvirt-gobject/libvirt-gobject-compat.c @@ -103,3 +103,19 @@ g_simple_async_report_take_gerror_in_idle(GObject *object, g_object_unref(simple); } #endif + +GMutex *gvir_mutex_new(void) +{ + GMutex *mutex; + + mutex = g_new(GMutex, 1); + g_mutex_init(mutex); + + return mutex; +} + +void gvir_mutex_free(GMutex *mutex) +{ + g_mutex_clear(mutex); + g_free(mutex); +} diff --git a/libvirt-gobject/libvirt-gobject-compat.h b/libvirt-gobject/libvirt-gobject-compat.h index 839dfe1..1d45018 100644 --- a/libvirt-gobject/libvirt-gobject-compat.h +++ b/libvirt-gobject/libvirt-gobject-compat.h @@ -27,8 +27,12 @@ #include <gio/gio.h> #if GLIB_CHECK_VERSION(2, 31, 0) -#define g_mutex_new() g_new0(GMutex, 1) -#define g_mutex_free(m) g_free(m) + +void gvir_mutex_free(GMutex *mutex); +GMutex *gvir_mutex_new(void); +#define g_mutex_new gvir_mutex_new +#define g_mutex_free gvir_mutex_free + #endif #if !GLIB_CHECK_VERSION(2,26,0) -- 1.8.0

2 1

[libvirt] [RFC PATCH 1/n] snapshot: add revert-and-create branching of external snapshots
by Eric Blake 14 Nov '12

14 Nov '12

Right now, libvirt refuses to revert to the state at the time of an external snapshot, because doing so would reset things so that the next time the domain boots, we are using the backing file; but modifying the backing file invalidates all qcow2 files that are based on top of it. There are three possibilities for lifting this restriction: 1. delete all snapshot metadata and qcow2 files that are invalidated by the revert (losing changes since the snapshot) 2. perform a block commit (such as with qemu-img commit) to merge the qcow2 file back into the backing file (keeping the changes since the snapshot) 3. create NEW qcow2 files that wrap the same base file as the original snapshot (keeping the changes since the original snapshot) This patch documents the API for option 3, by adding a new flag to virDomainSnapshotCreateXML (the only snapshot-related function that takes XML, which is required to pass in new file names during the branch), and wires up virsh to expose it. Later patches will enhance virDomainRevertToSnapshot to cover options 1 and 2. API wise, an application wishing to do the revert-and-create operation must add a <branch>name</branch> element to their <domainsnapshot> XML, and pass VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH in the flags argument. In virsh, snapshot-create gains a new boolean flag --branch that must match the XML passed in, and snapshot-create-as gains a new --branch=name argument along with a --current boolean for creating such XML. * include/libvirt/libvirt.h.in (VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH): New flag. * src/libvirt.c (virDomainSnapshotCreateXML): Document it, and enforce some mutual exclusion. (virDomainRevertToSnapshot): Mention how to revert to an external snapshot without having to delete snapshots. * docs/formatsnapshot.html.in: Document the new <branch> element. * docs/schemas/domainsnapshot.rng: Allow new element. * tools/virsh-snapshot.c (cmdSnapshotCreate): Add --branch option. (cmdSnapshotCreateAs): Likewise, also add --current. * tools/virsh.pod (snapshot-create, snapshot-create-as): Document new usage. --- Sending this patch now, to make sure I'm on the right track. I have the following plans for the next few patches: 1. Enhance virDomainSnapshotListFlags to add two new filter groups: VIR_DOMAIN_SNAPSHOT_LIST_OFFLINE VIR_DOMAIN_SNAPSHOT_LIST_ONLINE VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL (and possibly VIR_DOMAIN_SNAPSHOT_LIST_MIXED if we change our stance and allow mixing internal and external in one snapshot) 2. Add a flag to virDomainSnapshotParseFlags in src/conf/snapshot_conf.h; when present, the new element is parsed, and appropriate elements of the in-memory snapshot representation are copied in from the existing external snapshot. 3. Implement the new flag in qemu_driver.c for creation. Note that branching works for both disk-only (whether offline or online) and checkpoints - the new snapshot will share the same memory (none or external) as the original, and all that really changes is the creation (or reuse) of new backing files. 4. Think about ramifications on revert - right now, with no options, revert means to go back to the state where the snapshot was created; but now that we allow the creation of branches, and since the branches have external disk state which persists even when not on the branch, we need a new revert flag that says to revert to an external snapshots and its subsequent changes 5. Think about ramifications on delete - for example, if two branches of external snapshots share a common memory state file, and we delete only one of the two branches, the memory state file must not be deleted. docs/formatsnapshot.html.in | 23 +++++++++++++++++++---- docs/schemas/domainsnapshot.rng | 5 +++++ include/libvirt/libvirt.h.in | 3 +++ src/libvirt.c | 35 ++++++++++++++++++++++++++++++----- tools/virsh-snapshot.c | 31 ++++++++++++++++++++++++++----- tools/virsh.pod | 16 ++++++++++++++-- 6 files changed, 97 insertions(+), 16 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 8fcc04c..5018f41 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -87,7 +87,12 @@ sets that snapshot as current, and the prior current snapshot is the parent of the new snapshot. Branches in the hierarchy can be formed by reverting to a snapshot with a child, then creating - another snapshot. + another snapshot. In the case of external snapshots, modifying + the backing files would invalidate all external files that + depend on the backing file, so the action of reverting to a + snapshot must be accompanied by either a request to delete all + invalidated snapshots, or to create a new snapshot at the same + time as the revert. </p> <p> The top-level <code>domainsnapshot</code> element may contain @@ -188,9 +193,19 @@ </dd> <dt><code>parent</code></dt> <dd>The parent of this snapshot. If present, this element - contains exactly one child element, name. This specifies the - name of the parent snapshot of this snapshot, and is used to - represent trees of snapshots. Readonly. + contains exactly one child element, <code>name</code>. This + specifies the name of the parent snapshot of this snapshot, + and is used to represent trees of snapshots. Readonly. + </dd> + <dt><code>branch</code></dt> + <dd>The name of an existing external snapshot that forms the + branch point for this snapshot. Required when creating a + snapshot with + the <code>VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH</code> flag, and + not present on output. When creating a branch snapshot, the + same set of <code>disks</code> must be external, + and <code>memory</code> is copied from the branch + point. <span class="since">since 1.0.1</span>. </dd> <dt><code>domain</code></dt> <dd>The domain that this snapshot was taken against. Older diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 45d55b5..6721e8f 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -85,6 +85,11 @@ </element> </element> </optional> + <optional> + <element name='parent'> + <text/> + </element> + </optional> </interleave> </element> </define> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bf584a0..53d21ce 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3775,6 +3775,9 @@ typedef enum { VIR_DOMAIN_SNAPSHOT_CREATE_LIVE = (1 << 8), /* create the snapshot while the guest is running */ + VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH = (1 << 9), /* specify an existing + external snapshot as + the branching point */ } virDomainSnapshotCreateFlags; /* Take a snapshot of the current VM state */ diff --git a/src/libvirt.c b/src/libvirt.c index bcb8233..e88effc 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17747,9 +17747,7 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * normally fails if snapshot metadata still remains on the source * machine). When redefining snapshot metadata, the current snapshot * will not be altered unless the VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT - * flag is also present. It is an error to request the - * VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flag without - * VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE. On some hypervisors, + * flag is also present. On some hypervisors, * redefining an existing snapshot can be used to alter host-specific * portions of the domain XML to be used during revert (such as * backing filenames associated with disk devices), but must not alter @@ -17788,6 +17786,15 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * is not present, an error is thrown. Moreover, this flag requires * VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY to be passed as well. * + * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH, then @xmlDesc + * must describe an existing external snapshot as well as actions to + * create a new snapshot from the point where the existing snapshot was + * created rather than the current state of the guest. In this mode, + * the new snapshot branch is created but not activated unless the + * VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flag is also present. It is an + * error to request VIR_DOMAIN_SNAPSHOT_CREATE_LIVE or + * VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE in this mode. + * * By default, if the snapshot involves external files, and any of the * destination files already exist as a non-empty regular file, the * snapshot is rejected to avoid losing contents of those files. @@ -17844,9 +17851,11 @@ virDomainSnapshotCreateXML(virDomainPtr domain, } if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT) && - !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) { + !(flags & (VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | + VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH))) { virReportInvalidArg(flags, - _("use of 'current' flag in %s requires 'redefine' flag"), + _("use of 'current' flag in %s requires 'redefine' " + "or 'branch' flag"), __FUNCTION__); goto error; } @@ -17864,6 +17873,14 @@ virDomainSnapshotCreateXML(virDomainPtr domain, __FUNCTION__); goto error; } + if (!!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) + + !!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_LIVE) + + !!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH) > 1) { + virReportInvalidArg(flags, + _("'redefine', 'live', and 'branch' flags in %s are mutually exclusive"), + __FUNCTION__); + goto error; + } if (conn->driver->domainSnapshotCreateXML) { virDomainSnapshotPtr ret; @@ -18673,6 +18690,14 @@ error: * inactive snapshots with a @flags request to start the domain after * the revert. * + * Reverting to a snapshot with external state and then running the + * domain would invalidate all external files that depend on the backing + * file. Therefore, this function will fail unless any child external + * snapshots of the revert point first been deleted or merged. It is + * also possible to effectively revert and create a new snapshot branch + * in one operation, without invalidating snapshots, by using the + * VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH flag of virDomainSnapshotCreateXML(). + * * Returns 0 if the creation is successful, -1 on error. */ int diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 4281109..3de9878 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -120,7 +120,8 @@ static const vshCmdOptDef opts_snapshot_create[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"xmlfile", VSH_OT_DATA, 0, N_("domain snapshot XML")}, {"redefine", VSH_OT_BOOL, 0, N_("redefine metadata for existing snapshot")}, - {"current", VSH_OT_BOOL, 0, N_("with redefine, set current snapshot")}, + {"current", VSH_OT_BOOL, 0, + N_("with redefine or branch, set current snapshot")}, {"no-metadata", VSH_OT_BOOL, 0, N_("take snapshot but create no metadata")}, {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")}, {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")}, @@ -128,6 +129,7 @@ static const vshCmdOptDef opts_snapshot_create[] = { {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")}, {"atomic", VSH_OT_BOOL, 0, N_("require atomic operation")}, {"live", VSH_OT_BOOL, 0, N_("take a live snapshot")}, + {"branch", VSH_OT_BOOL, 0, N_("create a branch snapshot")}, {NULL, 0, 0, NULL} }; @@ -158,6 +160,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC; if (vshCommandOptBool(cmd, "live")) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE; + if (vshCommandOptBool(cmd, "branch")) + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) @@ -306,6 +310,10 @@ static const vshCmdOptDef opts_snapshot_create_as[] = { {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")}, {"atomic", VSH_OT_BOOL, 0, N_("require atomic operation")}, {"live", VSH_OT_BOOL, 0, N_("take a live snapshot")}, + {"branch", VSH_OT_DATA, VSH_OFLAG_REQ_OPT, + N_("name of external snapshot to use as a branch point")}, + {"current", VSH_OT_BOOL, 0, + N_("with branch, make the new snapshot current")}, {"memspec", VSH_OT_DATA, VSH_OFLAG_REQ_OPT, N_("memory attributes: [file=]name[,snapshot=type]")}, {"diskspec", VSH_OT_ARGV, 0, @@ -322,6 +330,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; const char *desc = NULL; const char *memspec = NULL; + const char *branch = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; unsigned int flags = 0; const vshCmdOpt *opt = NULL; @@ -340,6 +349,19 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC; if (vshCommandOptBool(cmd, "live")) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE; + if (vshCommandOptBool(cmd, "branch")) { + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH; + if (vshCommandOptString(cmd, "branch", &branch) < 0) { + vshError(ctl, _("argument must not be empty")); + goto cleanup; + } + if (vshCommandOptBool(cmd, "memspec")) { + vshError(ctl, _("--branch and --memspec are mutually exclusive")); + goto cleanup; + } + } + if (vshCommandOptBool(cmd, "current")) + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) @@ -352,10 +374,9 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) } virBufferAddLit(&buf, "<domainsnapshot>\n"); - if (name) - virBufferEscapeString(&buf, " <name>%s</name>\n", name); - if (desc) - virBufferEscapeString(&buf, " <description>%s</description>\n", desc); + virBufferEscapeString(&buf, " <name>%s</name>\n", name); + virBufferEscapeString(&buf, " <description>%s</description>\n", desc); + virBufferEscapeString(&buf, " <branch>%s</branch>\n", branch); if (vshCommandOptString(cmd, "memspec", &memspec) < 0 || vshParseSnapshotMemspec(ctl, &buf, memspec) < 0) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 0808d72..a79ed50 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2606,7 +2606,7 @@ used to represent properties of snapshots. =item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]] | [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external>] -[I<--quiesce>] [I<--atomic>] [I<--live>]} +[I<--quiesce>] [I<--atomic>] [I<--live>] [I<--branch>]} Create a snapshot for domain I<domain> with the properties specified in I<xmlfile>. Normally, the only properties settable for a domain snapshot @@ -2663,6 +2663,12 @@ If I<--live> is specified, libvirt takes the snapshot while the guest is running. This increases the size of the memory image of the external checkpoint. This is currently supported only for external checkpoints. +If I<--branch> is specified, then I<xmlfile> must contain a B<branch> +element naming an existing external snapshot which the new snapshot +will branch from. In this usage, the new snapshot must also be +external, and will not be made current unless I<--current> is also +requested. + Existence of snapshot metadata will prevent attempts to B<undefine> a persistent domain. However, for transient domains, snapshot metadata is silently lost when the domain quits running (whether @@ -2671,7 +2677,8 @@ by command such as B<destroy> or by internal guest action). =item B<snapshot-create-as> I<domain> {[I<--print-xml>] | [I<--no-metadata>] [I<--halt>] [I<--reuse-external>]} [I<name>] [I<description>] [I<--disk-only> [I<--quiesce>]] [I<--atomic>] -[[I<--live>] [I<--memspec> B<memspec>]] [I<--diskspec>] B<diskspec>]... +[[I<--live>] [I<--memspec> B<memspec>]] [I<--branch> B<branch> [I<--current>]] +[I<--diskspec>] B<diskspec>]... Create a snapshot for domain I<domain> with the given <name> and <description>; if either value is omitted, libvirt will choose a @@ -2687,6 +2694,11 @@ by a B<memspec> of the form B<[file=]name[,snapshot=type]>, where type can be B<none>, B<internal>, or B<external>. To include a literal comma in B<file=name>, escape it with a second comma. +The I<--branch> option can be used to create an external snapshot that +branches from an existing snapshot named B<branch>. In this usage, the +new snapshot is not made current unless I<--current> is also present. +Specifying a branch point is incompatible with using I<--memspec>. + The I<--diskspec> option can be used to control how I<--disk-only> and external checkpoints create external files. This option can occur multiple times, according to the number of <disk> elements in the domain -- 1.7.11.7

3 7

[libvirt] [libvirt-glib] docs: Specify correct function to free GVirDomainInfo
by Zeeshan Ali (Khattak) 14 Nov '12

14 Nov '12

From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org> --- libvirt-gobject/libvirt-gobject-domain.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 34d7519..4b82db9 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -957,8 +957,9 @@ gboolean gvir_domain_set_config(GVirDomain *domain, * gvir_domain_get_info: * @dom: the domain * - * Returns: (transfer full): the info. The returned object should be - * unreffed with g_object_unref() when no longer needed. + * Returns: (transfer full): the info. The returned structure should be + * freed using #g_boxed_free() with GVIR_TYPE_DOMAIN_INFO as the first argument + * when no longer needed. */ GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom, GError **err) -- 1.8.0

1 1

[libvirt] [libvirt-glib 1/4] gobject: don't try to use pool's volumes before a successful refresh
by Christophe Fergeau 14 Nov '12

14 Nov '12

gvir_storage_pool_refresh must be called and must be successful before trying to use gvir_storage_pool_get_volume, gvir_storage_pool_get_volumes and gvir_storage_pool_create_volume. As the storage pool refresh can fail for reasons external to libvirt/libvirt-gobject, the library user should check _refresh errors. This commit outputs runtime warnings when these functions are called and GVirObjectStoragePool::priv::volumes is NULL. --- libvirt-gobject/libvirt-gobject-storage-pool.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 7f50037..a4316e9 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -523,6 +523,8 @@ GList *gvir_storage_pool_get_volumes(GVirStoragePool *pool) if (priv->volumes != NULL) { volumes = g_hash_table_get_values(priv->volumes); g_list_foreach(volumes, gvir_storage_vol_ref, NULL); + } else { + g_warn_if_reached(); } g_mutex_unlock(priv->lock); @@ -548,9 +550,14 @@ GVirStorageVol *gvir_storage_pool_get_volume(GVirStoragePool *pool, priv = pool->priv; g_mutex_lock(priv->lock); - volume = g_hash_table_lookup(priv->volumes, name); - if (volume) - g_object_ref(volume); + if (priv->volumes != NULL) { + volume = g_hash_table_lookup(priv->volumes, name); + if (volume) + g_object_ref(volume); + } else { + g_warn_if_reached(); + volume = NULL; + } g_mutex_unlock(priv->lock); return volume; @@ -604,7 +611,14 @@ GVirStorageVol *gvir_storage_pool_create_volume } g_mutex_lock(priv->lock); - g_hash_table_insert(priv->volumes, g_strdup(name), volume); + if (priv->volumes != NULL) { + g_hash_table_insert(priv->volumes, g_strdup(name), volume); + } else { + g_warn_if_reached(); + g_object_unref(G_OBJECT(volume)); + g_mutex_unlock(priv->lock); + return NULL; + } g_mutex_unlock(priv->lock); return g_object_ref(volume); -- 1.7.12.1

2 11

[libvirt] [libvirt-glib] miscellaneous fixes
by Christophe Fergeau 14 Nov '12

14 Nov '12

This series does misc leak fixes in libvirt-glib, as well as an API doc fix, and a deprecation warning fix. Christophe

2 12

[libvirt] Is possible that cpu_maps.xml changed during different releases?
by Jiang, Yunhong 14 Nov '12

14 Nov '12

Hi, all I have two questions to the cpu_maps.xml in different releases, hope someone can give me some hints: a) Will it be possible that the features defined in cpu_maps.xml for one specific CPU model (like Nehalem) will be different? For example, one feature is not listed for Nehalem in release x.y, and added in release x.y+1? 2) Is the format of the cpu_maps.xml fine defined or will be it changed during releases? I asked this because currently the features defined in the capabilities only list features not included in the definition in cpu_maps.xml for the corresponding model. So if I want to get the full features supported by the host, I have to parse the capabilities and the cpu_maps.xml. I didn't find the definition for cpu_maps.xml format, although the capabilities format is well defined in http://libvirt.org/guide/html/Application_Development_Guide-Connections-Cap…. Thanks --jyh

2 4

[libvirt] [libvirt-glib] Introduce gvir_warning/gvir_critical
by Christophe Fergeau 14 Nov '12

14 Nov '12

They are similar to g_warning/g_critical, but also log the last libvirt error. They are meant to be called right after a libvirt call failed, in other cases g_warning/g_critical are still to be used. --- libvirt-glib/libvirt-glib-error.c | 34 +++++++++++++++++++++++ libvirt-glib/libvirt-glib-error.h | 2 ++ libvirt-glib/libvirt-glib.sym | 5 ++++ libvirt-gobject/libvirt-gobject-connection.c | 4 +-- libvirt-gobject/libvirt-gobject-domain-snapshot.c | 2 +- libvirt-gobject/libvirt-gobject-domain.c | 14 ++-------- libvirt-gobject/libvirt-gobject-interface.c | 2 +- libvirt-gobject/libvirt-gobject-network-filter.c | 17 ++++-------- libvirt-gobject/libvirt-gobject-network.c | 14 ++-------- libvirt-gobject/libvirt-gobject-node-device.c | 4 +-- libvirt-gobject/libvirt-gobject-secret.c | 11 ++------ libvirt-gobject/libvirt-gobject-storage-pool.c | 17 +++--------- libvirt-gobject/libvirt-gobject-storage-vol.c | 2 +- libvirt-gobject/libvirt-gobject-stream.c | 2 +- 14 files changed, 66 insertions(+), 64 deletions(-) diff --git a/libvirt-glib/libvirt-glib-error.c b/libvirt-glib/libvirt-glib-error.c index 7356aed..5c97da4 100644 --- a/libvirt-glib/libvirt-glib-error.c +++ b/libvirt-glib/libvirt-glib-error.c @@ -257,3 +257,37 @@ void gvir_set_error_valist(GError **error, g_free(message); } + +static void +gvir_log_valist(GLogLevelFlags level, const gchar *format, va_list args) +{ + gchar *message; + virErrorPtr verr = virGetLastError(); + + message = g_strdup_vprintf(format, args); + + if (verr) + g_log(G_LOG_DOMAIN, level, "%s: %s", message, verr->message); + else + g_log(G_LOG_DOMAIN, level, "%s", message); + + g_free(message); +} + +void gvir_warning(const gchar *format, ...) +{ + va_list args; + + va_start(args, format); + gvir_log_valist(G_LOG_LEVEL_WARNING, format, args); + va_end(args); +} + +void gvir_critical(const gchar *format, ...) +{ + va_list args; + + va_start(args, format); + gvir_log_valist(G_LOG_LEVEL_CRITICAL, format, args); + va_end(args); +} diff --git a/libvirt-glib/libvirt-glib-error.h b/libvirt-glib/libvirt-glib-error.h index 9e44383..1421047 100644 --- a/libvirt-glib/libvirt-glib-error.h +++ b/libvirt-glib/libvirt-glib-error.h @@ -59,6 +59,8 @@ void gvir_set_error_valist(GError **error, const gchar *format, va_list args); +void gvir_critical(const gchar *format, ...); +void gvir_warning(const gchar *format, ...); G_END_DECLS diff --git a/libvirt-glib/libvirt-glib.sym b/libvirt-glib/libvirt-glib.sym index 53b8907..d9ddb07 100644 --- a/libvirt-glib/libvirt-glib.sym +++ b/libvirt-glib/libvirt-glib.sym @@ -15,4 +15,9 @@ LIBVIRT_GLIB_0.0.7 { *; }; +LIBVIRT_GLIB_0.1.4 { + gvir_critical; + gvir_warning; +} LIBVIRT_GLIB_0.0.7; + # .... define new API here using predicted next version number .... diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 9628989..e6ccfd0 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -273,7 +273,7 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, GVirConnectionPrivate *priv = gconn->priv; if (virDomainGetUUIDString(dom, uuid) < 0) { - g_warning("Failed to get domain UUID on %p", dom); + gvir_warning("Failed to get domain UUID on %p", dom); return 0; } @@ -463,7 +463,7 @@ gboolean gvir_connection_open(GVirConnection *conn, } if (virConnectDomainEventRegister(priv->conn, domain_event_cb, conn, NULL) == -1) - g_warning("Failed to register domain events, ignoring"); + gvir_warning("Failed to register domain events, ignoring"); g_mutex_unlock(priv->lock); diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c b/libvirt-gobject/libvirt-gobject-domain-snapshot.c index 11a073c..ce60ea0 100644 --- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c +++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c @@ -167,7 +167,7 @@ const gchar *gvir_domain_snapshot_get_name(GVirDomainSnapshot *snapshot) const char *name; if (!(name = virDomainSnapshotGetName(priv->handle))) { - g_warning("Failed to get domain_snapshot name on %p", priv->handle); + gvir_warning("Failed to get domain_snapshot name on %p", priv->handle); return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8ade3ea..9d4e232 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -135,16 +135,8 @@ static void gvir_domain_constructed(GObject *object) G_OBJECT_CLASS(gvir_domain_parent_class)->constructed(object); /* xxx we may want to turn this into an initable */ - if (virDomainGetUUIDString(priv->handle, priv->uuid) < 0) { - virErrorPtr verr = virGetLastError(); - if (verr) { - g_warning("Failed to get domain UUID on %p: %s", - priv->handle, verr->message); - } else { - g_warning("Failed to get domain UUID on %p", - priv->handle); - } - } + if (virDomainGetUUIDString(priv->handle, priv->uuid) < 0) + gvir_warning("Failed to get domain UUID on %p", priv->handle); } @@ -292,7 +284,7 @@ const gchar *gvir_domain_get_name(GVirDomain *dom) priv = dom->priv; if (!(name = virDomainGetName(priv->handle))) { - g_warning("Failed to get domain name on %p", priv->handle); + gvir_warning("Failed to get domain name on %p", priv->handle); return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-interface.c b/libvirt-gobject/libvirt-gobject-interface.c index ecabb41..4523100 100644 --- a/libvirt-gobject/libvirt-gobject-interface.c +++ b/libvirt-gobject/libvirt-gobject-interface.c @@ -164,7 +164,7 @@ const gchar *gvir_interface_get_name(GVirInterface *iface) g_return_val_if_fail(GVIR_IS_INTERFACE(iface), NULL); if (!(name = virInterfaceGetName(iface->priv->handle))) { - g_warning("Failed to get interface name on %p", iface->priv->handle); + gvir_warning("Failed to get interface name on %p", iface->priv->handle); return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-network-filter.c b/libvirt-gobject/libvirt-gobject-network-filter.c index 00b1ed2..54c1eeb 100644 --- a/libvirt-gobject/libvirt-gobject-network-filter.c +++ b/libvirt-gobject/libvirt-gobject-network-filter.c @@ -118,16 +118,9 @@ static void gvir_network_filter_constructed(GObject *object) G_OBJECT_CLASS(gvir_network_filter_parent_class)->constructed(object); /* xxx we may want to turn this into an initable */ - if (virNWFilterGetUUIDString(priv->handle, priv->uuid) < 0) { - virErrorPtr verr = virGetLastError(); - if (verr) { - g_warning("Failed to get network filter UUID on %p: %s", - priv->handle, verr->message); - } else { - g_warning("Failed to get network filter UUID on %p", - priv->handle); - } - } + if (virNWFilterGetUUIDString(priv->handle, priv->uuid) < 0) + gvir_warning("Failed to get network filter UUID on %p", + priv->handle); } @@ -187,8 +180,8 @@ const gchar *gvir_network_filter_get_name(GVirNetworkFilter *filter) g_return_val_if_fail(GVIR_IS_NETWORK_FILTER(filter), NULL); if (!(name = virNWFilterGetName(filter->priv->handle))) { - g_warning("Failed to get network_filter name on %p", - filter->priv->handle); + gvir_warning("Failed to get network_filter name on %p", + filter->priv->handle); return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-network.c b/libvirt-gobject/libvirt-gobject-network.c index 9625ece..0f17ecc 100644 --- a/libvirt-gobject/libvirt-gobject-network.c +++ b/libvirt-gobject/libvirt-gobject-network.c @@ -117,16 +117,8 @@ static void gvir_network_constructed(GObject *object) G_OBJECT_CLASS(gvir_network_parent_class)->constructed(object); /* xxx we may want to turn this into an initable */ - if (virNetworkGetUUIDString(priv->handle, priv->uuid) < 0) { - virErrorPtr verr = virGetLastError(); - if (verr) { - g_warning("Failed to get network UUID on %p: %s", - priv->handle, verr->message); - } else { - g_warning("Failed to get network UUID on %p", - priv->handle); - } - } + if (virNetworkGetUUIDString(priv->handle, priv->uuid) < 0) + gvir_warning("Failed to get network UUID on %p", priv->handle); } static void gvir_network_class_init(GVirNetworkClass *klass) @@ -185,7 +177,7 @@ const gchar *gvir_network_get_name(GVirNetwork *network) g_return_val_if_fail(GVIR_IS_NETWORK(network), NULL); if (!(name = virNetworkGetName(network->priv->handle))) { - g_warning("Failed to get network name on %p", network->priv->handle); + gvir_warning("Failed to get network name on %p", network->priv->handle); return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-node-device.c b/libvirt-gobject/libvirt-gobject-node-device.c index eb314ed..781fc5d 100644 --- a/libvirt-gobject/libvirt-gobject-node-device.c +++ b/libvirt-gobject/libvirt-gobject-node-device.c @@ -164,8 +164,8 @@ const gchar *gvir_node_device_get_name(GVirNodeDevice *device) g_return_val_if_fail(GVIR_IS_NODE_DEVICE(device), NULL); if (!(name = virNodeDeviceGetName(device->priv->handle))) { - g_warning("Failed to get node_device name on %p", - device->priv->handle); + gvir_warning("Failed to get node_device name on %p", + device->priv->handle); return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-secret.c b/libvirt-gobject/libvirt-gobject-secret.c index 8bfee88..01b8ed1 100644 --- a/libvirt-gobject/libvirt-gobject-secret.c +++ b/libvirt-gobject/libvirt-gobject-secret.c @@ -118,15 +118,8 @@ static void gvir_secret_constructed(GObject *object) G_OBJECT_CLASS(gvir_secret_parent_class)->constructed(object); /* xxx we may want to turn this into an initable */ - if (virSecretGetUUIDString(priv->handle, priv->uuid) < 0) { - virErrorPtr verr = virGetLastError(); - if (verr) { - g_warning("Failed to get secret UUID on %p: %s", - priv->handle, verr->message); - } else { - g_warning("Failed to get secret UUID on %p", priv->handle); - } - } + if (virSecretGetUUIDString(priv->handle, priv->uuid) < 0) + gvir_warning("Failed to get secret UUID on %p", priv->handle); } diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index a09e5a7..adcdda7 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -128,15 +128,8 @@ static void gvir_storage_pool_constructed(GObject *object) G_OBJECT_CLASS(gvir_storage_pool_parent_class)->constructed(object); /* xxx we may want to turn this into an initable */ - if (virStoragePoolGetUUIDString(priv->handle, priv->uuid) < 0) { - virErrorPtr verr = virGetLastError(); - if (verr) { - g_warning("Failed to get storage pool UUID on %p: %s", - priv->handle, verr->message); - } else { - g_warning("Failed to get storage pool UUID on %p", priv->handle); - } - } + if (virStoragePoolGetUUIDString(priv->handle, priv->uuid) < 0) + gvir_warning("Failed to get storage pool UUID on %p", priv->handle); } @@ -215,10 +208,8 @@ const gchar *gvir_storage_pool_get_name(GVirStoragePool *pool) g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), NULL); - if (!(name = virStoragePoolGetName(pool->priv->handle))) { - g_warning("Failed to get storage_pool name on %p", pool->priv->handle); - return NULL; - } + if (!(name = virStoragePoolGetName(pool->priv->handle))) + gvir_warning("Failed to get storage_pool name on %p", pool->priv->handle); return name; } diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c index 9256445..c7ebb45 100644 --- a/libvirt-gobject/libvirt-gobject-storage-vol.c +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c @@ -200,7 +200,7 @@ const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol) g_return_val_if_fail(GVIR_IS_STORAGE_VOL(vol), NULL); if (!(name = virStorageVolGetName(vol->priv->handle))) { - g_warning("Failed to get storage_vol name on %p", vol->priv->handle); + gvir_warning("Failed to get storage_vol name on %p", vol->priv->handle); return NULL; } diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c index fa1a32c..f0e43d0 100644 --- a/libvirt-gobject/libvirt-gobject-stream.c +++ b/libvirt-gobject/libvirt-gobject-stream.c @@ -218,7 +218,7 @@ static void gvir_stream_finalize(GObject *object) gvir_stream_update_events(self); if (virStreamFinish(priv->handle) < 0) - g_critical("cannot finish stream"); + gvir_critical("cannot finish stream"); virStreamFree(priv->handle); } -- 1.7.12.1

2 3

[libvirt] [libvirt-glib] Add gvir_domain_update_device() and enums needed for it
by Alexander Larsson 14 Nov '12

14 Nov '12

This is a wrapper for virDomainUpdateDeviceFlags. --- libvirt-gobject/libvirt-gobject-domain.c | 42 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 17 +++++++++++++ libvirt-gobject/libvirt-gobject.sym | 7 ++++++ 3 files changed, 66 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8ade3ea..a00a042 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1428,6 +1428,48 @@ GList *gvir_domain_get_devices(GVirDomain *domain, return g_list_reverse (ret); } + +/** + * gvir_domain_update_device: + * @dom: the domain + * @device: A modified device config + * @flags: bitwise-OR of #GVirDomainUpdateDeviceFlags + * @err: (allow-none):Place-holder for error or NULL + * + * Update the configuration of a device. + * + * Returns: TRUE if device was updated successfully, FALSE otherwise. + */ +gboolean +gvir_domain_update_device(GVirDomain *dom, + GVirConfigDomainDevice *device, + guint flags, + GError **err) +{ + gchar *xml; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(device), FALSE); + + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(device)); + g_return_val_if_fail(xml != NULL, FALSE); + + if (virDomainUpdateDeviceFlags(dom->priv->handle, + xml, flags) < 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, + 0, + "Failed to update device"); + g_free (xml); + + return FALSE; + } + + g_free (xml); + return TRUE; +} + + /** * gvir_domain_create_snapshot: * @dom: the domain diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 6e7b10d..a622a46 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -158,6 +158,18 @@ typedef enum { GVIR_DOMAIN_SNAPSHOT_ATOMIC = VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC, } GVirDomainSnapshotCreateFlags; +/** + * GVirDomainUpdateDeviceFlags: + * @GVIR_DOMAIN_UPDATE_DEVICE_CURRENT: Update current domain state + * @GVIR_DOMAIN_UPDATE_DEVICE_LIVE: Update state for only active domains + * @GVIR_DOMAIN_UPDATE_DEVICE_CONFIG: Update state for persistent state only + */ +typedef enum { + GVIR_DOMAIN_UPDATE_DEVICE_CURRENT = VIR_DOMAIN_AFFECT_CURRENT, + GVIR_DOMAIN_UPDATE_DEVICE_LIVE = VIR_DOMAIN_AFFECT_LIVE, + GVIR_DOMAIN_UPDATE_DEVICE_CONFIG = VIR_DOMAIN_AFFECT_CONFIG, +} GVirDomainUpdateDeviceFlags; + typedef struct _GVirDomainInfo GVirDomainInfo; struct _GVirDomainInfo { @@ -294,6 +306,11 @@ gboolean gvir_domain_get_saved(GVirDomain *dom); GList *gvir_domain_get_devices(GVirDomain *domain, GError **err); +gboolean gvir_domain_update_device(GVirDomain *dom, + GVirConfigDomainDevice *device, + guint flags, + GError **err); + GVirDomainSnapshot * gvir_domain_create_snapshot(GVirDomain *dom, GVirConfigDomainSnapshot *custom_conf, diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 3a40a8a..2034e89 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -204,4 +204,11 @@ LIBVIRT_GOBJECT_0.1.3 { gvir_domain_wakeup_finish; } LIBVIRT_GOBJECT_0.1.2; +LIBVIRT_GOBJECT_0.1.4 { + global: + gvir_domain_update_device_flags_get_type; + gvir_domain_update_device; +} LIBVIRT_GOBJECT_0.1.3; + + # .... define new API here using predicted next version number .... -- 1.7.12.1

3 4