Re: [libvirt] [PATCH V2] storagevol: add nocow to vol xml
>>> On 7/9/2014 at 07:25 PM, in message <53bd26a7.9090...@redhat.com>, Ján Tomko wrote: > On 07/08/2014 08:47 AM, Chunyan Liu wrote: > > Add 'nocow' to storage volume xml so that user can have an option > > to set NOCOW flag to the newly created volume. It's useful on btrfs > > file system to enhance performance. > > > > Btrfs has low performance when hosting VM images, even more when the guest > > in those VM are also using btrfs as file system. One way to mitigate this > > bad performance is to turn off COW attributes on VM files. Generally, there > > are two ways to turn off NOCOW on btrfs: a) by mounting fs with nodatacow, > > s/turn off NOCOW/turn off COW/ > > > then all newly created files will be NOCOW. b) per file. Add the NOCOW file > > attribute. It could only be done to empty or new files. > > > > This patch tries the second way, according to 'nocow' option, it could set > > NOCOW flag per file: > > for raw file images, handle 'nocow' in libvirt code; for non-raw file > images, > > pass 'nocow=on' option to qemu-img, and let qemu-img to handle that > > (requires > > qemu-img version >= 2.1). > > > > Signed-off-by: Chunyan Liu > > --- > > Changes: > > - now qemu-img can handle 'nocow=on' option, just pass 'nocow=on' to > > qemu-img for non-raw file images. No need to handle all file type > > in libvirt code. > > > > Pervious version is here: > > http://www.redhat.com/archives/libvir-list/2013-December/msg01257.html > > > > --- > > docs/formatstorage.html.in| 7 +++ > > docs/schemas/storagevol.rng | 5 + > > Adding a test case to storagevolxml2argvtest would be nice. Also, by adding > the volume XML to storagevolxml2xmlin for this test, it will get validated > against the rng schema in 'storagevolschematest'. Thanks. Will add it. - Chunyan > > > src/conf/storage_conf.c | 3 +++ > > src/storage/storage_backend.c | 22 ++ > > src/util/virstoragefile.h | 1 + > > 5 files changed, 38 insertions(+) > > > > diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in > > index 1cd82b4..e8862bf 100644 > > --- a/docs/formatstorage.html.in > > +++ b/docs/formatstorage.html.in > > @@ -385,6 +385,7 @@ > > > > > >1.1 > > +> > > > > > @@ -424,6 +425,12 @@ > > 1.1 is used. If omitted, qemu-img default is used. > > Since 1.1.0 > > > > + nocow > > + Turn off COW of the newly created volume. So far, this is only > valid > > +to a file image in btrfs file system. It will improve performance > when > > s/to a file/for a file/ > > > +the file image is used in VM. To create non-raw file images, it > > +requires QEMU version since 2.1. Since > 1.2.6 > > 1.2.7 > > Otherwise looks good to me. > > Jan > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list> >
[libvirt] [PATCH] build: package .pc files for mingw64
Commit 65d8c92a fixed the mingw spec file for 32-bit builds,
but forgot to make the adjustment for 64-bit builds:
Checking for unpackaged file(s): /usr/lib/rpm/check-files
/home/eblake/rpmbuild/BUILDROOT/mingw-libvirt-1.2.7-1.fc20.eblake1404944503.x86_64
error: Installed (but unpackaged) file(s) found:
/usr/x86_64-w64-mingw32/sys-root/mingw/lib/pkgconfig/libvirt-lxc.pc
/usr/x86_64-w64-mingw32/sys-root/mingw/lib/pkgconfig/libvirt-qemu.pc
* mingw-libvirt.spec.in (%files): List missing .pc files.
Signed-off-by: Eric Blake
---
Pushing under the build-breaker rule, since I detected it via
./autobuild.sh. Also backporting to v1.2.6-maint.
mingw-libvirt.spec.in | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index a555f9c..a1e58cb 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -258,6 +258,8 @@ rm -rf
$RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
%{mingw64_libdir}/libvirt.dll.a
%{mingw64_libdir}/pkgconfig/libvirt.pc
+%{mingw64_libdir}/pkgconfig/libvirt-qemu.pc
+%{mingw64_libdir}/pkgconfig/libvirt-lxc.pc
%{mingw64_libdir}/libvirt-lxc.dll.a
%{mingw64_libdir}/libvirt-qemu.dll.a
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: fix gnulib build for mingw
Pavel flagged a build regression under mingw, and traced it to a recent flaw in gnulib for working around nl_langinfo. This picks up the fix. * gnulib: Update to latest, for mingw build fixes. Signed-off-by: Eric Blake --- Pushing under the build-breaker rule * .gnulib 9d5efe7...2d28074 (2): > nl_langinfo: fix build under mingw > mountlist: do not classify a bind-mounted dir entry as "dummy" .gnulib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gnulib b/.gnulib index 9d5efe7..2d28074 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 9d5efe7d61e1d96a6d7c97ffa4d21c5e33e9bd30 +Subproject commit 2d280742a9e30088aa169f53353765d5daafe4c0 -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] libxl: add discard support to libxl_device_disk
Olaf Hering wrote:
> Translate libvirt discard settings into libxl-4.5 discard settings.
>
> Signed-off-by: Olaf Hering
> ---
> v3:
> passing discard= with old libxl is now a fatal error
> v2:
> add cast to switch variable to let compiler check if the code handles
> all enum values
>
Thanks for addressing the comments.
> src/libxl/libxl_conf.c | 31 +++
> 1 file changed, 31 insertions(+)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 8eeaf82..538cfa4 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -715,6 +715,35 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> return -1;
> }
>
> +static int
> +libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
> +{
> +if (!x_disk->readwrite)
> +return 0;
> +#if defined(LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE)
> +switch ((enum virDomainDiskDiscard)discard) {
> +case VIR_DOMAIN_DISK_DISCARD_DEFAULT:
> +case VIR_DOMAIN_DISK_DISCARD_LAST:
> +break;
> +case VIR_DOMAIN_DISK_DISCARD_UNMAP:
> +libxl_defbool_set(&x_disk->discard_enable, true);
> +break;
> +case VIR_DOMAIN_DISK_DISCARD_IGNORE:
> +libxl_defbool_set(&x_disk->discard_enable, false);
> +break;
> +}
> +return 0;
> +#else
> +if (discard == VIR_DOMAIN_DISK_DISCARD_DEFAULT)
> +return 0;
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("This version of libxenlight does not support "
> + "disk 'discard' option passing"));
>
Fails 'make syntax-check', but otherwise ACK. I squashed in the below
fix and pushed.
Regards,
Jim
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 22ab902..0b4a0b5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -736,7 +736,7 @@ libxlDiskSetDiscard(libxl_device_disk *x_disk, int
discard)
#else
if (discard == VIR_DOMAIN_DISK_DISCARD_DEFAULT)
return 0;
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("This version of libxenlight does not support "
"disk 'discard' option passing"));
return -1;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib] [PATCH v5 1/3] libvirt-gobject-domain: Add _fetch_snapshots
This function can be used to fetch the snapshots of a domain (according
to the given GVirDomainSnapshotListFlags) and save them in a
domain-internal GHashTable. A function to access them from outside will
be added in a later patch.
---
libvirt-gobject/libvirt-gobject-domain.c | 86
libvirt-gobject/libvirt-gobject-domain.h | 37 ++
libvirt-gobject/libvirt-gobject.sym | 2 +
3 files changed, 125 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c
b/libvirt-gobject/libvirt-gobject-domain.c
index c6e30e5..adb5179 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -38,6 +38,8 @@ struct _GVirDomainPrivate
{
virDomainPtr handle;
gchar uuid[VIR_UUID_STRING_BUFLEN];
+GHashTable *snapshots;
+GMutex *lock;
};
G_DEFINE_TYPE(GVirDomain, gvir_domain, G_TYPE_OBJECT);
@@ -121,6 +123,11 @@ static void gvir_domain_finalize(GObject *object)
g_debug("Finalize GVirDomain=%p", domain);
+if (priv->snapshots) {
+g_hash_table_unref(priv->snapshots);
+}
+g_mutex_free(priv->lock);
+
virDomainFree(priv->handle);
G_OBJECT_CLASS(gvir_domain_parent_class)->finalize(object);
@@ -237,6 +244,7 @@ static void gvir_domain_init(GVirDomain *domain)
g_debug("Init GVirDomain=%p", domain);
domain->priv = GVIR_DOMAIN_GET_PRIVATE(domain);
+domain->priv->lock = g_mutex_new();
}
typedef struct virDomain GVirDomainHandle;
@@ -1514,3 +1522,81 @@ gvir_domain_create_snapshot(GVirDomain *dom,
g_free(custom_xml);
return dom_snapshot;
}
+
+
+
+/**
+ * gvir_domain_fetch_snapshots:
+ * @dom: The domain
+ * @list_flags: bitwise-OR of #GVirDomainSnapshotListFlags
+ * @cancellable: (allow-none)(transfer-none): cancellation object
+ * @error: (allow-none): Place-holder for error or NULL
+ *
+ * Returns: TRUE on success, FALSE otherwise.
+ */
+gboolean gvir_domain_fetch_snapshots(GVirDomain *dom,
+ guint list_flags,
+ GCancellable *cancellable,
+ GError **error)
+{
+GVirDomainPrivate *priv;
+virDomainSnapshotPtr *snapshots = NULL;
+GVirDomainSnapshot *snap;
+GHashTable *snap_table;
+int n_snaps = 0;
+int i;
+gboolean ret = TRUE;
+
+g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
+g_return_val_if_fail((error == NULL) || (*error == NULL), FALSE);
+
+priv = dom->priv;
+
+snap_table = g_hash_table_new_full(g_str_hash,
+ g_str_equal,
+ NULL,
+ g_object_unref);
+
+
+n_snaps = virDomainListAllSnapshots(priv->handle, &snapshots, list_flags);
+
+if (g_cancellable_set_error_if_cancelled(cancellable, error)) {
+ret = FALSE;
+goto cleanup;
+}
+
+if (n_snaps < 0) {
+gvir_set_error(error, GVIR_DOMAIN_ERROR, 0,
+ "Unable to fetch snapshots of %s",
+ gvir_domain_get_name(dom));
+ret = FALSE;
+goto cleanup;
+}
+
+for (i = 0; i < n_snaps; i ++) {
+if (g_cancellable_set_error_if_cancelled(cancellable, error)) {
+ret = FALSE;
+goto cleanup;
+}
+snap = GVIR_DOMAIN_SNAPSHOT(g_object_new(GVIR_TYPE_DOMAIN_SNAPSHOT,
+ "handle", snapshots[i],
+ NULL));
+g_hash_table_insert(snap_table,
+(gpointer)gvir_domain_snapshot_get_name(snap),
+snap);
+}
+
+
+g_mutex_lock(priv->lock);
+if (priv->snapshots != NULL)
+g_hash_table_unref(priv->snapshots);
+priv->snapshots = snap_table;
+snap_table = NULL;
+g_mutex_unlock(priv->lock);
+
+cleanup:
+free(snapshots);
+if (snap_table != NULL)
+g_hash_table_unref (snap_table);
+return ret;
+}
diff --git a/libvirt-gobject/libvirt-gobject-domain.h
b/libvirt-gobject/libvirt-gobject-domain.h
index 38d3458..8c1a8e5 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ -183,6 +183,39 @@ typedef enum {
GVIR_DOMAIN_REBOOT_GUEST_AGENT= VIR_DOMAIN_REBOOT_GUEST_AGENT,
} GVirDomainRebootFlags;
+/**
+ * GVirDomainSnapshotListFlags:
+ * @GVIR_DOMAIN_SNAPSHOT_LIST_ALL: List all snapshots
+ * @GVIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS: List all descendants, not just
+ * children, when listing a snapshot.
+ * For historical reasons, groups do
not use contiguous bits.
+ * @GVIR_DOMAIN_SNAPSHOT_LIST_ROOTS: Filter by snapshots with no parents, when
listing a domain
+ * @GVIR_DOMAIN_SNAPSHOT_LIST_METADATA: Filter by snapshots which have metadata
+ * @GVIR_DOMAIN_SNAPSHOT_LIST_LEAVES: Filter by snapshots wi
[libvirt] [libvirt-glib] [PATCH v5 3/3] GVirDomain: Add async version of _fetch_snapshots
---
libvirt-gobject/libvirt-gobject-domain.c | 61
libvirt-gobject/libvirt-gobject-domain.h | 10 ++
libvirt-gobject/libvirt-gobject.sym | 2 ++
3 files changed, 73 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c
b/libvirt-gobject/libvirt-gobject-domain.c
index 8f48c2e..7fd5043 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -1621,3 +1621,64 @@ GList *gvir_domain_get_snapshots(GVirDomain *dom)
return snapshots;
}
+
+
+
+static void _fetch_snapshots_async_thread(GTask *task,
+ gpointer source_object,
+ gpointer task_data,
+ GCancellable *cancellable) {
+GError *error = NULL;
+gboolean status;
+
+status = gvir_domain_fetch_snapshots(source_object,
+ GPOINTER_TO_UINT(task_data),
+ cancellable,
+ &error);
+if (status)
+g_task_return_boolean(task, TRUE);
+else
+g_task_return_error(task, error);
+}
+
+
+/**
+ *
+ * @dom: The domain
+ * @list_flags: bitwise-OR of #GVirDomainSnapshotListFlags
+ * @cancellable: (allow-none)(transfer-none): cancellation object
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
+ */
+void gvir_domain_fetch_snapshots_async(GVirDomain *dom,
+ guint list_flags,
+ GCancellable *cancellable,
+ GAsyncReadyCallback callback,
+ gpointer user_data) {
+GTask *task;
+
+g_return_if_fail(GVIR_IS_DOMAIN(dom));
+g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
+
+task = g_task_new(dom, cancellable, callback, user_data);
+g_task_set_task_data(task, GUINT_TO_POINTER(list_flags), NULL);
+g_task_run_in_thread(task, _fetch_snapshots_async_thread);
+g_object_unref(task);
+}
+
+
+/**
+ * gvir_domain_fetch_snapshots_finish:
+ * @dom: a #GVirDomain
+ * @res: (transfer none): async method result
+ *
+ * Returns: TRUE on success, FALSE otherwise.
+ */
+gboolean gvir_domain_fetch_snapshots_finish(GVirDomain *dom,
+GAsyncResult *res,
+GError **error) {
+g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
+g_return_val_if_fail(g_task_is_valid(res, dom), FALSE);
+
+return g_task_propagate_boolean(G_TASK(res), error);
+}
diff --git a/libvirt-gobject/libvirt-gobject-domain.h
b/libvirt-gobject/libvirt-gobject-domain.h
index 22870c1..9846375 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ -370,6 +370,16 @@ gboolean gvir_domain_fetch_snapshots(GVirDomain *dom,
GList *gvir_domain_get_snapshots(GVirDomain *dom);
+void gvir_domain_fetch_snapshots_async(GVirDomain *dom,
+ guint list_flags,
+ GCancellable *cancellable,
+ GAsyncReadyCallback callback,
+ gpointer user_data);
+
+gboolean gvir_domain_fetch_snapshots_finish(GVirDomain *dom,
+GAsyncResult *res,
+GError **error);
+
G_END_DECLS
diff --git a/libvirt-gobject/libvirt-gobject.sym
b/libvirt-gobject/libvirt-gobject.sym
index 28e547a..6aa8b86 100644
--- a/libvirt-gobject/libvirt-gobject.sym
+++ b/libvirt-gobject/libvirt-gobject.sym
@@ -237,6 +237,8 @@ LIBVIRT_GOBJECT_0.1.5 {
LIBVIRT_GOBJECT_0.1.9 {
global:
gvir_domain_fetch_snapshots;
+ gvir_domain_fetch_snapshots_async;
+ gvir_domain_fetch_snapshots_finish;
gvir_domain_get_snapshots;
gvir_domain_snapshot_delete;
gvir_domain_snapshot_list_flags_get_type;
--
2.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib] [PATCH v5 2/3] libvirt-gobject-domain: Add _get_snapshots
... which returns a GList of GVirDomainSnapshots, i.e. without any tree
structure or other relationship between the snapshots.
---
libvirt-gobject/libvirt-gobject-domain.c | 21 +
libvirt-gobject/libvirt-gobject-domain.h | 4
libvirt-gobject/libvirt-gobject.sym | 1 +
3 files changed, 26 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c
b/libvirt-gobject/libvirt-gobject-domain.c
index adb5179..8f48c2e 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -1600,3 +1600,24 @@ cleanup:
g_hash_table_unref (snap_table);
return ret;
}
+
+/**
+ * gvir_domain_get_snapshots:
+ * @dom: The domain
+ * Returns: (element-type LibvirtGObject.DomainSnapshot) (transfer full): A
+ * list of all the snapshots available for the given domain. The returned
+ * list should be freed with g_list_free(), after its elements have been
+ * unreffed with g_object_unref().
+ */
+GList *gvir_domain_get_snapshots(GVirDomain *dom)
+{
+GList *snapshots = NULL;
+g_return_val_if_fail(GVIR_IS_DOMAIN(dom), NULL);
+
+if (dom->priv->snapshots != NULL) {
+snapshots = g_hash_table_get_values(dom->priv->snapshots);
+g_list_foreach(snapshots, (GFunc)g_object_ref, NULL);
+}
+
+return snapshots;
+}
diff --git a/libvirt-gobject/libvirt-gobject-domain.h
b/libvirt-gobject/libvirt-gobject-domain.h
index 8c1a8e5..22870c1 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ -367,6 +367,10 @@ gboolean gvir_domain_fetch_snapshots(GVirDomain *dom,
guint list_flags,
GCancellable *cancellable,
GError **error);
+
+GList *gvir_domain_get_snapshots(GVirDomain *dom);
+
+
G_END_DECLS
#endif /* __LIBVIRT_GOBJECT_DOMAIN_H__ */
diff --git a/libvirt-gobject/libvirt-gobject.sym
b/libvirt-gobject/libvirt-gobject.sym
index 781310f..28e547a 100644
--- a/libvirt-gobject/libvirt-gobject.sym
+++ b/libvirt-gobject/libvirt-gobject.sym
@@ -237,6 +237,7 @@ LIBVIRT_GOBJECT_0.1.5 {
LIBVIRT_GOBJECT_0.1.9 {
global:
gvir_domain_fetch_snapshots;
+ gvir_domain_get_snapshots;
gvir_domain_snapshot_delete;
gvir_domain_snapshot_list_flags_get_type;
} LIBVIRT_GOBJECT_0.1.5;
--
2.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib] [PATCH v4 3/3] GVirDomain: Add async version of _fetch_snapshots
On 07.07, Christophe Fergeau wrote:
> Hey,
>
> On Mon, Jun 30, 2014 at 07:50:16PM +0200, Timm Bäder wrote:
> > ---
> > libvirt-gobject/libvirt-gobject-domain.c | 60
> >
> > libvirt-gobject/libvirt-gobject-domain.h | 10 ++
> > libvirt-gobject/libvirt-gobject.sym | 2 ++
> > 3 files changed, 72 insertions(+)
> >
> > diff --git a/libvirt-gobject/libvirt-gobject-domain.c
> > b/libvirt-gobject/libvirt-gobject-domain.c
> > index b12a4a0..6b53d25 100644
> > --- a/libvirt-gobject/libvirt-gobject-domain.c
> > +++ b/libvirt-gobject/libvirt-gobject-domain.c
> > @@ -1618,3 +1618,63 @@ GList *gvir_domain_get_snapshots(GVirDomain *dom)
> >
> > return snapshots;
> > }
> > +
> > +
> > +
> > +static void _fetch_snapshots_async_thread(GTask *task,
> > + gpointer source_object,
> > + gpointer task_data,
> > + GCancellable *cancellable) {
> > +GError *error = NULL;
> > +gboolean status;
> > +
> > +status = gvir_domain_fetch_snapshots(source_object,
> > + GPOINTER_TO_UINT(task_data),
> > + cancellable,
> > + &error);
> > +if (status)
> > +g_task_return_boolean(task, TRUE);
> > +else
> > +g_task_return_error(task, error);
> > +}
> > +
> > +
> > +/**
> > + *
> > + * @dom: The domain
> > + * @list_flags: bitwise-OR of #GVirDomainSnapshotListFlags
> > + * @cancellable: (allow-none)(transfer-none): cancellation object
> > + * @callback: (scope async): completion callback
> > + * @user_data: (closure): opaque data for callback
> > + */
> > +void gvir_domain_fetch_snapshots_async(GVirDomain *dom,
> > + guint list_flags,
> > + GCancellable *cancellable,
> > + GAsyncReadyCallback callback,
> > + gpointer user_data) {
> > +GTask *task;
> > +
> > +g_return_if_fail(GVIR_IS_DOMAIN(dom));
> > +g_return_if_fail((cancellable == NULL) ||
> > G_IS_CANCELLABLE(cancellable));
> > +
> > +task = g_task_new(dom, cancellable, callback, user_data);
>
> GTask was introduced in glib 2.36, so configure.ac needs to be updated
> to reflect that. glib 2.36 was released in March 2013, and is available
> in RHEL7, so using this would be fine with me. Others may want to
> disagree though :)
>
> > +g_task_set_task_data(task, GUINT_TO_POINTER(list_flags), NULL);
> > +g_task_run_in_thread(task, _fetch_snapshots_async_thread);
>
> I'm a bit unclear as how the initial ref obtained through g_task_new()
> is handled. Have you checked it's correctly freed after _finish() has
> been called and that it's not leaked?
Ouch, I've been fooled by the example in the GTask documentation (which
was wrong), it does ineed have to be unreffed in the function that calls
g_task_run_in_thread (which will itself take its own ref).
>
> Looks good otherwise.
>
> Christophe
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib] [PATCH v4 2/3] libvirt-gobject-domain: Add _get_snapshots
On 07.07, Christophe Fergeau wrote: > Hey, > > On Mon, Jun 30, 2014 at 07:50:15PM +0200, Timm Bäder wrote: > > ... which returns a GList of GVirDomainSnapshots, i.e. without any tree > > structure or other relationship between the snapshots. > > Looks good, ACK. Any plans to return the snapshots as a tree at some > point? Or can the library user reconstruct this tree information from > the current API? > > Christophe I don't really have plans on this (since I don't need it) but since there's gvir_config_domain_snapshot_get_parent exists I guess it can be done manually (although not very convenient...). Timm -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Rework lxc apparmor profile
Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default.
This profile allows quite a lot, but strives to restrict access to
dangerous resources.
Removing the explicit authorizations to bash, systemd and cron files,
forces them to keep the lxc profile for all applications inside the
container. PUx permissions where leading to running systemd (and others
tasks) unconfined.
Put the generic files, network and capabilities restrictions directly
in the TEMPLATE.lxc: this way, users can restrict them on a per
container basis.
---
examples/apparmor/Makefile.am | 6 +-
examples/apparmor/TEMPLATE.lxc| 15
examples/apparmor/{TEMPLATE => TEMPLATE.qemu} | 2 +-
examples/apparmor/libvirt-lxc | 119 +++---
src/security/security_apparmor.c | 20 +++--
src/security/virt-aa-helper.c | 29 +--
6 files changed, 148 insertions(+), 43 deletions(-)
create mode 100644 examples/apparmor/TEMPLATE.lxc
rename examples/apparmor/{TEMPLATE => TEMPLATE.qemu} (75%)
diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am
index a741e94..7a20e16 100644
--- a/examples/apparmor/Makefile.am
+++ b/examples/apparmor/Makefile.am
@@ -15,7 +15,8 @@
##
[libvirt] [PATCH 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
--- src/security/virt-aa-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..d563b98 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1342,7 +1342,8 @@ main(int argc, char **argv) vah_info(include_file); vah_info(included_files); rc = 0; -} else if ((rc = update_include_file(include_file, +} else if (ctl->def->virtType != VIR_DOMAIN_VIRT_LXC && + (rc = update_include_file(include_file, included_files, ctl->append)) != 0) goto cleanup; -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] AppArmor lxc profile fixes
Hi all,
Here are 2 patches fixing AppArmor profiles for lxc containers. The main
problem was
that the current profile was:
1/ too restricting as it needed to allow all needed applications
2/ used PUx permissions, which made systemd (or bash) run as unprofiled as
they
have no profiles defined.
The new profile is based on container-default profile shipped for lxc on Ubuntu.
All applications are now running under the parent profile (ix permission) and
some
critical files accesses are denied.
The first patch also avoid writing the useless libvirt-UUID.files for lxc
containers.
Cédric Bosdonnat (2):
Don't output libvirt-UUID.files for LXC apparmor profiles
Rework lxc apparmor profile
examples/apparmor/Makefile.am | 6 +-
examples/apparmor/TEMPLATE.lxc| 15
examples/apparmor/{TEMPLATE => TEMPLATE.qemu} | 2 +-
examples/apparmor/libvirt-lxc | 119 +++---
src/security/security_apparmor.c | 20 +++--
src/security/virt-aa-helper.c | 32 ++-
6 files changed, 150 insertions(+), 44 deletions(-)
create mode 100644 examples/apparmor/TEMPLATE.lxc
rename examples/apparmor/{TEMPLATE => TEMPLATE.qemu} (75%)
--
1.8.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't error out when cgroups don't exist
On 07/09/2014 02:15 AM, Martin Kletzander wrote: > When creating cgroups for vcpu and emulator threads whilst starting a > domain, we explicitly skip creating those cgroups in case priv->cgroup > is NULL (cgroups not supported) because SetAffinity() serves the same > purpose. If the host supports only some cgroups (the ones we need are > either unmounted or disabled in qemu.conf), we error out with weird > message even though we could continue starting the domain. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1097028 > > Signed-off-by: Martin Kletzander > --- > src/qemu/qemu_cgroup.c | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c > index 3394c68..0af6ac5 100644 > --- a/src/qemu/qemu_cgroup.c > +++ b/src/qemu/qemu_cgroup.c > @@ -949,7 +949,11 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) > virCgroupFree(&cgroup_vcpu); > } > > -return -1; > +if (period || quota) > +return -1; > + > +virResetLastError(); > +return 0; This still leaks the error message to the log, even if we proceed with execution. Checking up front is better than clearing errors after the fact, if that is possible. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] check for cfg->spiceTLS earlier in qemuProcessSPICEAllocatePorts
On 07/09/2014 03:09 AM, Ján Tomko wrote: > This saves a few lines of code and catches the error when: > > > > is specified with spice_tls = 0 in qemu.conf. > > Instead of this error in qemuBuildGraphicsSPICECommandLine: > error: unsupported configuration: spice secure channels set in XML > configuration, but TLS port is not provided > > an error is reported in qemuProcessSPICEAllocatePorts: > error: unsupported configuration: Auto allocation of spice TLS port > requested but spice TLS is disabled in qemu.conf > > Inspired by: > https://www.redhat.com/archives/libvir-list/2014-June/msg01408.html > --- > src/qemu/qemu_process.c | 33 +++-- > 1 file changed, 11 insertions(+), 22 deletions(-) ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: storage: Fix build after 25924dec0f9329d429aadae14e273602307e2214
On 07/09/14 15:01, Eric Blake wrote: > On 07/09/2014 02:05 AM, Peter Krempa wrote: >> The commit referenced above changed function arguments of >> virStorageFileGetMetadataFromBuf() but didn't tweak the >> ATTRIBUTE_NONNULL tied to them. This was caught by coverity as it >> actually obeys them. We disabled them for GCC and thus it didn't show >> up. > > Yeah, because gcc 4.8 still misbehaves on the attribute (silently > "optimizing" code without flagging invalid uses). There's been talk on > the gcc list about fixing it, although probably no sooner than 4.10 > timeframe. > >> >> Additionally in commit 3ea661deeabadc3c114dfb6f662b9fd17d714a01 I passed >> NULL to the backingFormat argument which was also marked as nonnull. Use >> a dummy int's address when the argument isn't supplied so that the code >> doesn't need to change much. >> --- >> src/util/virstoragefile.c | 4 >> src/util/virstoragefile.h | 3 +-- >> 2 files changed, 5 insertions(+), 2 deletions(-) > > ACK. > Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: storage: Fix build after 25924dec0f9329d429aadae14e273602307e2214
On 07/09/2014 02:05 AM, Peter Krempa wrote: > The commit referenced above changed function arguments of > virStorageFileGetMetadataFromBuf() but didn't tweak the > ATTRIBUTE_NONNULL tied to them. This was caught by coverity as it > actually obeys them. We disabled them for GCC and thus it didn't show > up. Yeah, because gcc 4.8 still misbehaves on the attribute (silently "optimizing" code without flagging invalid uses). There's been talk on the gcc list about fixing it, although probably no sooner than 4.10 timeframe. > > Additionally in commit 3ea661deeabadc3c114dfb6f662b9fd17d714a01 I passed > NULL to the backingFormat argument which was also marked as nonnull. Use > a dummy int's address when the argument isn't supplied so that the code > doesn't need to change much. > --- > src/util/virstoragefile.c | 4 > src/util/virstoragefile.h | 3 +-- > 2 files changed, 5 insertions(+), 2 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] virSecurityDeviceLabelDef: substitute 'norelabel' with 'relabel'
Similarly to the previous commit, boolean variables should not start
with 'no-' prefix.
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 12 ++--
src/security/security_dac.c | 8
src/security/security_selinux.c | 10 +-
src/util/virseclabel.c | 2 +-
src/util/virseclabel.h | 2 +-
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f6743e5..f75c0cb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4803,9 +4803,9 @@
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
relabel = virXMLPropString(list[i], "relabel");
if (relabel != NULL) {
if (STREQ(relabel, "yes")) {
-seclabels[i]->norelabel = false;
+seclabels[i]->relabel = true;
} else if (STREQ(relabel, "no")) {
-seclabels[i]->norelabel = true;
+seclabels[i]->relabel = false;
} else {
virReportError(VIR_ERR_XML_ERROR,
_("invalid security relabel value %s"),
@@ -4815,7 +4815,7 @@
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
}
VIR_FREE(relabel);
} else {
-seclabels[i]->norelabel = false;
+seclabels[i]->relabel = true;
}
/* labelskip is only parsed on live images */
@@ -4830,7 +4830,7 @@
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
seclabels[i]->label = label;
-if (label && seclabels[i]->norelabel) {
+if (label && !seclabels[i]->relabel) {
virReportError(VIR_ERR_XML_ERROR,
_("Cannot specify a label if relabelling is "
"turned off. model=%s"),
@@ -14736,7 +14736,7 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf,
{
/* For offline output, skip elements that allow labels but have no
* label specified (possible if labelskip was ignored on input). */
-if ((flags & VIR_DOMAIN_XML_INACTIVE) && !def->label && !def->norelabel)
+if ((flags & VIR_DOMAIN_XML_INACTIVE) && !def->label && def->relabel)
return;
virBufferAddLit(buf, "labelskip)
virBufferAddLit(buf, " labelskip='yes'");
else
-virBufferAsprintf(buf, " relabel='%s'", def->norelabel ? "no" : "yes");
+virBufferAsprintf(buf, " relabel='%s'", def->relabel ? "yes" : "no");
if (def->label) {
virBufferAddLit(buf, ">\n");
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 665cbc9..4d2a9d6 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -312,7 +312,7 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr
mgr,
disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
SECURITY_DAC_NAME);
-if (disk_seclabel && disk_seclabel->norelabel)
+if (disk_seclabel && !disk_seclabel->relabel)
return 0;
if (disk_seclabel && disk_seclabel->label) {
@@ -374,7 +374,7 @@
virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
SECURITY_DAC_NAME);
-if (disk_seclabel && disk_seclabel->norelabel)
+if (disk_seclabel && !disk_seclabel->relabel)
return 0;
/* If we have a shared FS and are doing migration, we must not change
@@ -703,7 +703,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
SECURITY_DAC_NAME);
-if (chr_seclabel && chr_seclabel->norelabel)
+if (chr_seclabel && !chr_seclabel->relabel)
return 0;
if (chr_seclabel && chr_seclabel->label) {
@@ -772,7 +772,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
SECURITY_DAC_NAME);
-if (chr_seclabel && chr_seclabel->norelabel)
+if (chr_seclabel && !chr_seclabel->relabel)
return 0;
switch ((virDomainChrType) dev_source->type) {
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 8d4a9aa..a0e89b7 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1130,7 +1130,7 @@
virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
SECURITY_SELINUX_NAME);
-if (!seclabel->relabel || (disk_seclabel && disk_secl
[libvirt] [PATCH 1/3] virSecurityLabelDef: substitute 'norelabel' with 'relabel'
This negation in names of boolean variables is driving me insane. The
code is much more readable if we drop the 'no-' prefix. Well, at least
for me.
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 20 ++--
src/security/security_apparmor.c | 10 +-
src/security/security_dac.c | 14 +++---
src/security/security_manager.c | 2 +-
src/security/security_selinux.c | 24 ++--
src/util/virseclabel.h | 2 +-
6 files changed, 34 insertions(+), 38 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b80e7cf..f6743e5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4576,9 +4576,9 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p != NULL) {
if (STREQ(p, "yes")) {
-def->norelabel = false;
+def->relabel = true;
} else if (STREQ(p, "no")) {
-def->norelabel = true;
+def->relabel = false;
} else {
virReportError(VIR_ERR_XML_ERROR,
_("invalid security relabel value %s"), p);
@@ -4587,13 +4587,13 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
}
VIR_FREE(p);
if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
-def->norelabel) {
+!def->relabel) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("dynamic label type must use resource
relabeling"));
goto error;
}
if (def->type == VIR_DOMAIN_SECLABEL_NONE &&
-!def->norelabel) {
+def->relabel) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("resource relabeling is not compatible with
'none' label type"));
goto error;
@@ -4601,9 +4601,9 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
} else {
if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
def->type == VIR_DOMAIN_SECLABEL_NONE)
-def->norelabel = true;
+def->relabel = false;
else
-def->norelabel = false;
+def->relabel = true;
}
/* Always parse model */
@@ -4635,7 +4635,7 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
}
/* Only parse imagelabel, if requested live XML with relabeling */
-if (!def->norelabel &&
+if (def->relabel &&
(!(flags & VIR_DOMAIN_XML_INACTIVE) &&
def->type != VIR_DOMAIN_SECLABEL_NONE)) {
p = virXPathStringLimit("string(./imagelabel[1])",
@@ -4793,7 +4793,7 @@
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
}
/* Can't use overrides if top-level doesn't allow relabeling. */
-if (vmDef && vmDef->norelabel) {
+if (vmDef && !vmDef->relabel) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("label overrides require relabeling to be "
"enabled at the domain level"));
@@ -14708,14 +14708,14 @@ virSecurityLabelDefFormat(virBufferPtr buf,
}
virBufferAsprintf(buf, " relabel='%s'",
- def->norelabel ? "no" : "yes");
+ def->relabel ? "yes" : "no");
if (def->label || def->imagelabel || def->baselabel) {
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
virBufferEscapeString(buf, "%s\n",
def->label);
-if (!def->norelabel)
+if (def->relabel)
virBufferEscapeString(buf, "%s\n",
def->imagelabel);
if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 1e2a38b..9603c78 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -281,7 +281,7 @@ reload_profile(virSecurityManagerPtr mgr,
if (!secdef)
return rc;
-if (secdef->norelabel)
+if (!secdef->relabel)
return 0;
if ((profile_name = get_profile_name(def)) == NULL)
@@ -481,7 +481,7 @@ AppArmorSetSecurityAllLabel(virSecurityManagerPtr mgr,
if (!secdef)
return -1;
-if (secdef->norelabel)
+if (!secdef->relabel)
return 0;
/* Reload the profile if stdin_path is specified. Note that
@@ -718,7 +718,7 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
if (!(secdef = virDomainDefGetSecurityLabelDef(def,
SECURITY_APPARMOR_NAME)))
return -1;
-if (secdef->norelabel)
+if (!secdef->relabel)
return 0;
if (secdef->imagelabel) {
@@ -805,7 +805,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
if (!secdef)
return -1;
-if (secdef->norelabel)
+if (!secdef->relabel)
return 0;
if (dev->m
[libvirt] [PATCH 3/3] conf: Disallow
https://bugzilla.redhat.com/show_bug.cgi?id=1113860
The combination of type='none' and relabel='yes' makes no sense as
'none' type basically means relabel='no'.
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f75c0cb..4215565 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4614,8 +4614,14 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
/* For the model 'none' none of the following labels is going to be
* present. Hence, return now. */
-if (STREQ_NULLABLE(def->model, "none"))
+if (STREQ_NULLABLE(def->model, "none")) {
+if (def->relabel) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("model 'none' does not allow relabeling"));
+goto error;
+}
return def;
+}
/* Only parse label, if using static labels, or
* if the 'live' VM XML is requested
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Couple of seclabels improvements
Yes, the first two patches basically revert 693eac388f1759d. Only the last one fixes a real problem. Michal Privoznik (3): virSecurityLabelDef: substitute 'norelabel' with 'relabel' virSecurityDeviceLabelDef: substitute 'norelabel' with 'relabel' conf: Disallow src/conf/domain_conf.c | 40 +++- src/security/security_apparmor.c | 10 +- src/security/security_dac.c | 22 +++--- src/security/security_manager.c | 2 +- src/security/security_selinux.c | 32 ++-- src/util/virseclabel.c | 2 +- src/util/virseclabel.h | 4 ++-- 7 files changed, 57 insertions(+), 55 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] conf: Improve metadata type verification
On 07/09/14 14:52, Ján Tomko wrote: > On 07/09/2014 02:44 PM, Peter Krempa wrote: >> Split out checking of invalid metadata type from the switch statement so >> that we can use the typecasted enum value to allow tracking addition of >> new items by the compliler. >> >> Also avoids two dead-code break statements. >> --- >> Version 2: >> - move the check back to the original function so that we don't break >> forward compat >> >> src/conf/domain_conf.c | 22 ++ >> 1 file changed, 14 insertions(+), 8 deletions(-) > > ACK > Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: document the possibility of accepting integers for numatune mode
On 07/09/2014 10:15 AM, Martin Kletzander wrote: > According to the code, 'virsh numatune' supports integers for > specifying --mode as well as the string definitions "strict", > "interleave", and "preferred". However, this possibility was not > documented anywhere, so this patch adds it to both the man page and > command help. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1085706 > > Signed-off-by: Martin Kletzander > --- > tools/virsh-domain.c | 3 ++- > tools/virsh.pod | 8 +--- > 2 files changed, 7 insertions(+), 4 deletions(-) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] conf: Improve metadata type verification
On 07/09/2014 02:44 PM, Peter Krempa wrote: > Split out checking of invalid metadata type from the switch statement so > that we can use the typecasted enum value to allow tracking addition of > new items by the compliler. > > Also avoids two dead-code break statements. > --- > Version 2: > - move the check back to the original function so that we don't break forward > compat > > src/conf/domain_conf.c | 22 ++ > 1 file changed, 14 insertions(+), 8 deletions(-) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't error out when cgroups don't exist
On 07/09/2014 02:30 PM, Martin Kletzander wrote: > On Wed, Jul 09, 2014 at 01:07:47PM +0200, Ján Tomko wrote: >> On 07/09/2014 10:15 AM, Martin Kletzander wrote: >>> When creating cgroups for vcpu and emulator threads whilst starting a >>> domain, we explicitly skip creating those cgroups in case priv->cgroup >>> is NULL (cgroups not supported) because SetAffinity() serves the same >>> purpose. If the host supports only some cgroups (the ones we need are >>> either unmounted or disabled in qemu.conf), we error out with weird >>> message even though we could continue starting the domain. >> >> Yet this patch does not change the error message. >> > > Yes, because there should be no error. > >>> >>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1097028 >>> >>> Signed-off-by: Martin Kletzander >>> --- >>> src/qemu/qemu_cgroup.c | 12 ++-- >>> 1 file changed, 10 insertions(+), 2 deletions(-) >>> >>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c >>> index 3394c68..0af6ac5 100644 >>> --- a/src/qemu/qemu_cgroup.c >>> +++ b/src/qemu/qemu_cgroup.c >>> @@ -949,7 +949,11 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) >>> virCgroupFree(&cgroup_vcpu); >>> } >>> >>> -return -1; >>> +if (period || quota) >>> +return -1; >>> + >>> +virResetLastError(); >>> +return 0; >>> } >>> >> >> This also resets OOM errors and errors that happen when these controllers are >> mounted. >> >> Can't we just check upfront if the needed controllers are available? >> > > There might be more problems than the controllers not being mounted, > but OK, the others are corner cases anyway. Would the following diff > be more suitable? Yes, that is much nicer. > > diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c > index 3394c68..d4c969b 100644 > --- a/src/qemu/qemu_cgroup.c > +++ b/src/qemu/qemu_cgroup.c > @@ -893,6 +893,15 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) > return -1; > } > > +/* > + * If CPU cgroup controller is not initialized here, than we need > + * neither period nor quota settings. And if CPUSET controller is > + * not initialized either, than there's nothing to do anyway. > + */ > +if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && > +!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) > +return 0; > + > /* We are trying to setup cgroups for CPU pinning, which can also > be done > * with virProcessSetAffinity, thus the lack of cgroups is not > fatal here. > */ > @@ -972,6 +981,15 @@ qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, > return -1; > } > > +/* > + * If CPU cgroup controller is not initialized here, than we need > + * neither period nor quota settings. And if CPUSET controller is > + * not initialized either, than there's nothing to do anyway. > + */ > +if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && > +!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) > +return 0; > + > if (priv->cgroup == NULL) > return 0; /* Not supported, so claim success */ > s/than/then/ in the comments. ACK with that fixed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] conf: Improve metadata type verification
Split out checking of invalid metadata type from the switch statement so
that we can use the typecasted enum value to allow tracking addition of
new items by the compliler.
Also avoids two dead-code break statements.
---
Version 2:
- move the check back to the original function so that we don't break forward
compat
src/conf/domain_conf.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b80e7cf..fa76eb4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19575,6 +19575,12 @@ virDomainObjGetMetadata(virDomainObjPtr vm,
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, NULL);
+if (type >= VIR_DOMAIN_METADATA_LAST) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("unknown metadata type '%d'"), type);
+goto cleanup;
+}
+
if (virDomainLiveConfigHelperMethod(caps, xmlopt, vm, &flags, &def) < 0)
goto cleanup;
@@ -19601,10 +19607,7 @@ virDomainObjGetMetadata(virDomainObjPtr vm,
goto cleanup;
break;
-default:
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("unknown metadata type"));
-goto cleanup;
+case VIR_DOMAIN_METADATA_LAST:
break;
}
@@ -19630,6 +19633,12 @@ virDomainDefSetMetadata(virDomainDefPtr def,
char *tmp;
int ret = -1;
+if (type >= VIR_DOMAIN_METADATA_LAST) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("unknown metadata type '%d'"), type);
+goto cleanup;
+}
+
switch ((virDomainMetadataType) type) {
case VIR_DOMAIN_METADATA_DESCRIPTION:
if (VIR_STRDUP(tmp, metadata) < 0)
@@ -19683,10 +19692,7 @@ virDomainDefSetMetadata(virDomainDefPtr def,
}
break;
-default:
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("unknown metadata type"));
-goto cleanup;
+case VIR_DOMAIN_METADATA_LAST:
break;
}
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't error out when cgroups don't exist
On Wed, Jul 09, 2014 at 01:07:47PM +0200, Ján Tomko wrote: On 07/09/2014 10:15 AM, Martin Kletzander wrote: When creating cgroups for vcpu and emulator threads whilst starting a domain, we explicitly skip creating those cgroups in case priv->cgroup is NULL (cgroups not supported) because SetAffinity() serves the same purpose. If the host supports only some cgroups (the ones we need are either unmounted or disabled in qemu.conf), we error out with weird message even though we could continue starting the domain. Yet this patch does not change the error message. Yes, because there should be no error. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1097028 Signed-off-by: Martin Kletzander --- src/qemu/qemu_cgroup.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3394c68..0af6ac5 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -949,7 +949,11 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) virCgroupFree(&cgroup_vcpu); } -return -1; +if (period || quota) +return -1; + +virResetLastError(); +return 0; } This also resets OOM errors and errors that happen when these controllers are mounted. Can't we just check upfront if the needed controllers are available? There might be more problems than the controllers not being mounted, but OK, the others are corner cases anyway. Would the following diff be more suitable? diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3394c68..d4c969b 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -893,6 +893,15 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) return -1; } +/* + * If CPU cgroup controller is not initialized here, than we need + * neither period nor quota settings. And if CPUSET controller is + * not initialized either, than there's nothing to do anyway. + */ +if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && +!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) +return 0; + /* We are trying to setup cgroups for CPU pinning, which can also be done * with virProcessSetAffinity, thus the lack of cgroups is not fatal here. */ @@ -972,6 +981,15 @@ qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, return -1; } +/* + * If CPU cgroup controller is not initialized here, than we need + * neither period nor quota settings. And if CPUSET controller is + * not initialized either, than there's nothing to do anyway. + */ +if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && +!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) +return 0; + if (priv->cgroup == NULL) return 0; /* Not supported, so claim success */ -- Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] storagevol: add nocow to vol xml
On 07/08/2014 08:47 AM, Chunyan Liu wrote: > Add 'nocow' to storage volume xml so that user can have an option > to set NOCOW flag to the newly created volume. It's useful on btrfs > file system to enhance performance. > > Btrfs has low performance when hosting VM images, even more when the guest > in those VM are also using btrfs as file system. One way to mitigate this > bad performance is to turn off COW attributes on VM files. Generally, there > are two ways to turn off NOCOW on btrfs: a) by mounting fs with nodatacow, s/turn off NOCOW/turn off COW/ > then all newly created files will be NOCOW. b) per file. Add the NOCOW file > attribute. It could only be done to empty or new files. > > This patch tries the second way, according to 'nocow' option, it could set > NOCOW flag per file: > for raw file images, handle 'nocow' in libvirt code; for non-raw file images, > pass 'nocow=on' option to qemu-img, and let qemu-img to handle that (requires > qemu-img version >= 2.1). > > Signed-off-by: Chunyan Liu > --- > Changes: > - now qemu-img can handle 'nocow=on' option, just pass 'nocow=on' to > qemu-img for non-raw file images. No need to handle all file type > in libvirt code. > > Pervious version is here: > http://www.redhat.com/archives/libvir-list/2013-December/msg01257.html > > --- > docs/formatstorage.html.in| 7 +++ > docs/schemas/storagevol.rng | 5 + Adding a test case to storagevolxml2argvtest would be nice. Also, by adding the volume XML to storagevolxml2xmlin for this test, it will get validated against the rng schema in 'storagevolschematest'. > src/conf/storage_conf.c | 3 +++ > src/storage/storage_backend.c | 22 ++ > src/util/virstoragefile.h | 1 + > 5 files changed, 38 insertions(+) > > diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in > index 1cd82b4..e8862bf 100644 > --- a/docs/formatstorage.html.in > +++ b/docs/formatstorage.html.in > @@ -385,6 +385,7 @@ > > >1.1 > +> > > @@ -424,6 +425,12 @@ > 1.1 is used. If omitted, qemu-img default is used. > Since 1.1.0 > > + nocow > + Turn off COW of the newly created volume. So far, this is only > valid > +to a file image in btrfs file system. It will improve performance > when s/to a file/for a file/ > +the file image is used in VM. To create non-raw file images, it > +requires QEMU version since 2.1. Since > 1.2.6 1.2.7 Otherwise looks good to me. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list>
Re: [libvirt] [PATCH] qemu: don't error out when cgroups don't exist
On 07/09/2014 10:15 AM, Martin Kletzander wrote: > When creating cgroups for vcpu and emulator threads whilst starting a > domain, we explicitly skip creating those cgroups in case priv->cgroup > is NULL (cgroups not supported) because SetAffinity() serves the same > purpose. If the host supports only some cgroups (the ones we need are > either unmounted or disabled in qemu.conf), we error out with weird > message even though we could continue starting the domain. Yet this patch does not change the error message. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1097028 > > Signed-off-by: Martin Kletzander > --- > src/qemu/qemu_cgroup.c | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c > index 3394c68..0af6ac5 100644 > --- a/src/qemu/qemu_cgroup.c > +++ b/src/qemu/qemu_cgroup.c > @@ -949,7 +949,11 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) > virCgroupFree(&cgroup_vcpu); > } > > -return -1; > +if (period || quota) > +return -1; > + > +virResetLastError(); > +return 0; > } > This also resets OOM errors and errors that happen when these controllers are mounted. Can't we just check upfront if the needed controllers are available? Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Build failed in Jenkins: libvirt-syntax-check #2465
On 07/08/2014 03:38 PM, Eric Blake wrote: > [adding bug-gnulib] > > On 07/08/2014 08:05 AM, Guido Günther wrote: >> On Tue, Jul 08, 2014 at 03:49:02PM +0200, Jenkins CI wrote: >>> 0.43 prohibit_empty_lines_at_EOF >>> prohibit_error_without_use >>> grep: write error >>> grep: write error >>> /bin/sed: couldn't write 25 items to stdout: Broken pipe >> >> I've disabled notifications to the list until I'm clear what the >> problem is. Sorry for the noise. > > That was not the cause of the buildbot reporting failure, but it IS > noisy. I suspect it is because your buildbot is running in an > environment with SIGPIPE ignored (and sadly, there is NO way from shell > to turn SIGPIPE back to normal if the shell inherits it as ignored). > Since this noise is occurring in a stock 'make syntax-check' from > gnulib, we ought to patch gnulib to work around noisy tools that don't > tolerate SIGPIPE being ignored. Note anything python 2 based could have this issue: https://bugzilla.redhat.com/show_bug.cgi?id=887194 Until that's fixed you need workarounds with sub processes. Search for SIGPIPE here for example: http://www.pixelbeat.org/libs/subProcess.py Pádraig. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC][scale] new API for querying domains stats
On Wed, Jul 09, 2014 at 06:14:12AM -0400, Francesco Romani wrote: > > > - Original Message - > > From: "Francesco Romani" > > To: libvir-list@redhat.com > > Sent: Friday, July 4, 2014 6:44:07 PM > > Subject: Re: [libvirt] [RFC][scale] new API for querying domains stats > > > > > However, a question here about bulk APIs. > > > > One cornerstone of oVirt is shared storage (NFS, ISCSI...); another is > > > > qemu/kvm, > > > > and COW images are supported (probably even the default, need to check). > > > > > > > > Due to storage being unavailable because a network outage, it happened > > > > that > > > > virDomainGetBlockInfo blocked beyond recover. > > > > > > > > On such scenarios, how will a bulk API behave? There will be a timeout > > > > or > > > > something else? > > > > > > It depends on the storage and the way it is configured. If NFS is mounted > > > with 'hard' + 'nointr' any call libvirt makes to dead storage will get > > > stuck in an uninterruptable sleep in kernel space. There's no way for > > > libvirt to time out since by the very definition of 'hard' mount option > > > it does not time out. If you mount with 'soft' then the calls libvirt > > > makes will time out. > > > > My bad, I worded poorly my question. > > > > What I mean is: on top of what the kernel or QEMU (libnfs, libiscsi) does, > > there are plans for any additional mechanism/safeguard? > > (I guess no, I'm asking just to be sure). > > Hi, > > maybe borderline offtopic, but still about blocking calls: > > We (VDSM/oVirt developers) are reviewing our usage of libvirt in sampling. > Afer a (quick) inspection of the code, I believe the following calls cannot > block due to FS/storage issues, as they do not need it in any way > > I'm quite confident about these > * virDomainGetCPUStats: uses cgroups only (no FS/storage access) > * virDomainInterfaceStats: uses /proc/net/dev (no FS/storage access) > * virDomainGetVcpus: uses uses /proc and syscall for PCPU affinity (no > FS/storage access) > * virDomainSchedulerParameters: which uses cgroups (no FS/storage access) > > Not sure about this, but it looks to me they don't need to access FS/storage > either: > * virDomainGetVcpusFlags > * virDomainGetMetadata > > > Can please anyone confirm or deny? If there is a prior call to libvirt that involves that guest domain which has blocked on storage, then this can prevent subsequent calls from completely since the prior call may hold a lock. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC][scale] new API for querying domains stats
- Original Message - > From: "Francesco Romani" > To: libvir-list@redhat.com > Sent: Friday, July 4, 2014 6:44:07 PM > Subject: Re: [libvirt] [RFC][scale] new API for querying domains stats > > > However, a question here about bulk APIs. > > > One cornerstone of oVirt is shared storage (NFS, ISCSI...); another is > > > qemu/kvm, > > > and COW images are supported (probably even the default, need to check). > > > > > > Due to storage being unavailable because a network outage, it happened > > > that > > > virDomainGetBlockInfo blocked beyond recover. > > > > > > On such scenarios, how will a bulk API behave? There will be a timeout or > > > something else? > > > > It depends on the storage and the way it is configured. If NFS is mounted > > with 'hard' + 'nointr' any call libvirt makes to dead storage will get > > stuck in an uninterruptable sleep in kernel space. There's no way for > > libvirt to time out since by the very definition of 'hard' mount option > > it does not time out. If you mount with 'soft' then the calls libvirt > > makes will time out. > > My bad, I worded poorly my question. > > What I mean is: on top of what the kernel or QEMU (libnfs, libiscsi) does, > there are plans for any additional mechanism/safeguard? > (I guess no, I'm asking just to be sure). Hi, maybe borderline offtopic, but still about blocking calls: We (VDSM/oVirt developers) are reviewing our usage of libvirt in sampling. Afer a (quick) inspection of the code, I believe the following calls cannot block due to FS/storage issues, as they do not need it in any way I'm quite confident about these * virDomainGetCPUStats: uses cgroups only (no FS/storage access) * virDomainInterfaceStats: uses /proc/net/dev (no FS/storage access) * virDomainGetVcpus: uses uses /proc and syscall for PCPU affinity (no FS/storage access) * virDomainSchedulerParameters: which uses cgroups (no FS/storage access) Not sure about this, but it looks to me they don't need to access FS/storage either: * virDomainGetVcpusFlags * virDomainGetMetadata Can please anyone confirm or deny? Thanks and best regards -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 22/28] util: storage: Return complete parent info from virStorageFileChainLookup
On 07/09/14 01:28, Eric Blake wrote: > On 07/04/2014 05:29 AM, Peter Krempa wrote: >> Instead of just returning the parent path, return the complete parent >> source structure. >> --- >> src/qemu/qemu_driver.c| 16 - >> src/util/virstoragefile.c | 17 -- >> src/util/virstoragefile.h | 2 +- >> tests/virstoragetest.c| 86 >> ++- >> 4 files changed, 56 insertions(+), 65 deletions(-) > >> @@ -15585,10 +15585,9 @@ qemuDomainBlockCommit(virDomainPtr dom, >> clean_access = true; >> if (qemuDomainPrepareDiskChainElement(driver, vm, disk, baseSource, >>VIR_DISK_CHAIN_READ_WRITE) < 0 || >> -(top_parent && top_parent != disk->src->path && >> - qemuDomainPrepareDiskChainElementPath(driver, vm, disk, >> - top_parent, >> - VIR_DISK_CHAIN_READ_WRITE) < >> 0)) >> +(top_parent != disk->src && >> + qemuDomainPrepareDiskChainElement(driver, vm, disk, top_parent, >> + VIR_DISK_CHAIN_READ_WRITE) < 0)) > > Oops. This doesn't quite work with active commit (where top_parent is > NULL, but where qemuDomainPrepareDiskChainElement isn't too happy to get > a NULL pointer). But then again, I'm rebasing my active commit on top > of you, so your patch is not wrong if it goes in first. If you want, > restore the condition to '(top_parent && top_parent != disk->src &&...)'. I went with the active_commit tolerant approach as the code was actually there before this patch. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 27/28] qemu: snapshot: Refactor image labelling of new snapshot files
On 07/09/14 03:59, Eric Blake wrote: > On 07/04/2014 05:29 AM, Peter Krempa wrote: >> Now that cgroups/security driver/locking driver support labelling of >> individual images and tolerate network storage we don't have to refrain >> from passing all image files to it. This allows to remove checking code > > s/to remove/removing the/ > >> as we already make sure that the snapshot function won't be called with >> unsupported options. >> --- >> src/qemu/qemu_driver.c | 62 >> +++--- >> 1 file changed, 18 insertions(+), 44 deletions(-) >> > > ACK > Thanks for the reviews. I've fixed all the points in the individual reviews and pushed this series until here. I'll need to repost the two remaining patches as they'll require some changes. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix domxml-to-native failing when spice_tls is not enabled
On 06/27/2014 04:37 PM, Jincheng Miao wrote: > The default graphics channel mode is 'any', so as to defaultMode attribute. > If defaultMode and channel mode are all the default value 'any', > qemuConnectDomainXMLToNative will set TLSPort. > But in qemuBuildGraphicsSPICECommandLine, if spice_tls is not enabled, > libvirtd > will report an error to tell the user that spice TLS is disabled in qemu.conf. > > So qemuConnectDomainXMLToNative should check spice_tls is enabled, > then decide to allocate an tlsPort number to this graphics. > > If user specified defaultMode is 'secure', qemuConnectDomainXMLToNative > could allocate tlsPort, and then let qemuBuildGraphicsSPICECommandLine reports > the spice_tls disabled error. > > The related bug is: > https://bugzilla.redhat.com/show_bug.cgi?id=1113868 > > Signed-off-by: Jincheng Miao > --- > src/qemu/qemu_driver.c |3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > ACK, I will push this later. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] check for cfg->spiceTLS earlier in qemuProcessSPICEAllocatePorts
This saves a few lines of code and catches the error when:
is specified with spice_tls = 0 in qemu.conf.
Instead of this error in qemuBuildGraphicsSPICECommandLine:
error: unsupported configuration: spice secure channels set in XML
configuration, but TLS port is not provided
an error is reported in qemuProcessSPICEAllocatePorts:
error: unsupported configuration: Auto allocation of spice TLS port
requested but spice TLS is disabled in qemu.conf
Inspired by:
https://www.redhat.com/archives/libvir-list/2014-June/msg01408.html
---
src/qemu/qemu_process.c | 33 +++--
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 570960e..ccc571b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3534,7 +3534,8 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
break;
case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY:
-needTLSPort = true;
+if (cfg->spiceTLS)
+needTLSPort = true;
needPort = true;
break;
}
@@ -3552,28 +3553,16 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
if (needTLSPort || graphics->data.spice.tlsPort == -1) {
if (!cfg->spiceTLS) {
-/* log an error and fail if tls was specifically
- * requested, or simply ignore (don't allocate a port)
- * if we're here due to "defaultMode='any'"
- * (aka unspecified).
- */
-if ((graphics->data.spice.tlsPort == -1) ||
-(graphics->data.spice.defaultMode
- == VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Auto allocation of spice TLS port requested "
- "but spice TLS is disabled in qemu.conf"));
-goto error;
-}
-} else {
-/* cfg->spiceTLS *is* in place, so it makes sense to
- * allocate a port.
- */
-if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0)
-goto error;
-
-graphics->data.spice.tlsPort = tlsPort;
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Auto allocation of spice TLS port requested "
+ "but spice TLS is disabled in qemu.conf"));
+goto error;
}
+
+if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0)
+goto error;
+
+graphics->data.spice.tlsPort = tlsPort;
}
return 0;
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: storage: Fix build after 25924dec0f9329d429aadae14e273602307e2214
On 07/09/14 10:12, Michal Privoznik wrote: > On 09.07.2014 10:05, Peter Krempa wrote: >> The commit referenced above changed function arguments of >> virStorageFileGetMetadataFromBuf() but didn't tweak the >> ATTRIBUTE_NONNULL tied to them. This was caught by coverity as it >> actually obeys them. We disabled them for GCC and thus it didn't show >> up. >> >> Additionally in commit 3ea661deeabadc3c114dfb6f662b9fd17d714a01 I passed >> NULL to the backingFormat argument which was also marked as nonnull. Use >> a dummy int's address when the argument isn't supplied so that the code >> doesn't need to change much. >> --- >> src/util/virstoragefile.c | 4 >> src/util/virstoragefile.h | 3 +-- >> 2 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h >> index 744a6ba..528cfad 100644 >> --- a/src/util/virstoragefile.h >> +++ b/src/util/virstoragefile.h >> @@ -301,8 +301,7 @@ virStorageSourcePtr >> virStorageFileGetMetadataFromBuf(const char *path, >>size_t len, >>int format, >>int >> *backingFormat) >> -ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) >> -ATTRIBUTE_NONNULL(5); >> +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); >> int virStorageFileChainGetBroken(virStorageSourcePtr chain, >>char **broken_file); >> > > So if we need to cripple passing NULL, why do we even have > ATTRIBUTE_NONNULL? Wouldn't it be simpler if > virStorageFileGetMetadataFromBuf would accept NULL? This patch actually removes the ATTRIBUTE_NONNULL from two places. Argument 4, as it's now an int and the attribute doesn't make sense there. And argument 5 as we are passing NULL in one place now. Therefore I also added the null tolerance code as the underlying code doesn't tolerate NULL. In virStorageFileGetMetadataFromFD we do the same stuff. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: document the possibility of accepting integers for numatune mode
According to the code, 'virsh numatune' supports integers for
specifying --mode as well as the string definitions "strict",
"interleave", and "preferred". However, this possibility was not
documented anywhere, so this patch adds it to both the man page and
command help.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1085706
Signed-off-by: Martin Kletzander
---
tools/virsh-domain.c | 3 ++-
tools/virsh.pod | 8 +---
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b5b9f91..5a17aff 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7823,7 +7823,8 @@ static const vshCmdOptDef opts_numatune[] = {
},
{.name = "mode",
.type = VSH_OT_DATA,
- .help = N_("NUMA mode, one of strict, preferred and interleave")
+ .help = N_("NUMA mode, one of strict, preferred and interleave \n"
+"or a number from the virDomainNumatuneMemMode enum")
},
{.name = "nodeset",
.type = VSH_OT_DATA,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 99d0b74..a5e8406 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1402,9 +1402,11 @@ Set or get a domain's numa parameters, corresponding to
the
element of domain XML. Without flags, the current settings are
displayed.
-I can be one of `strict', `interleave' and `preferred'. For a
-running domain, the mode can't be changed, and the nodeset can be
-changed only if the domain was started with a mode of `strict'.
+I can be one of `strict', `interleave' and `preferred' or any
+valid number from the virDomainNumatuneMemMode enum in case the daemon
+supports it. For a running domain, the mode can't be changed, and the
+nodeset can be changed only if the domain was started with a mode of
+`strict'.
I is a list of numa nodes used by the host for running the domain.
Its syntax is a comma separated list, with '-' for ranges and '^' for
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: don't error out when cgroups don't exist
When creating cgroups for vcpu and emulator threads whilst starting a domain, we explicitly skip creating those cgroups in case priv->cgroup is NULL (cgroups not supported) because SetAffinity() serves the same purpose. If the host supports only some cgroups (the ones we need are either unmounted or disabled in qemu.conf), we error out with weird message even though we could continue starting the domain. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1097028 Signed-off-by: Martin Kletzander --- src/qemu/qemu_cgroup.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3394c68..0af6ac5 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -949,7 +949,11 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) virCgroupFree(&cgroup_vcpu); } -return -1; +if (period || quota) +return -1; + +virResetLastError(); +return 0; } int @@ -1016,7 +1020,11 @@ qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, virCgroupFree(&cgroup_emulator); } -return -1; +if (period || quota) +return -1; + +virResetLastError(); +return 0; } int -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: storage: Fix build after 25924dec0f9329d429aadae14e273602307e2214
On 09.07.2014 10:05, Peter Krempa wrote:
The commit referenced above changed function arguments of
virStorageFileGetMetadataFromBuf() but didn't tweak the
ATTRIBUTE_NONNULL tied to them. This was caught by coverity as it
actually obeys them. We disabled them for GCC and thus it didn't show
up.
Additionally in commit 3ea661deeabadc3c114dfb6f662b9fd17d714a01 I passed
NULL to the backingFormat argument which was also marked as nonnull. Use
a dummy int's address when the argument isn't supplied so that the code
doesn't need to change much.
---
src/util/virstoragefile.c | 4
src/util/virstoragefile.h | 3 +--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 505b652..de0e27a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -955,6 +955,10 @@ virStorageFileGetMetadataFromBuf(const char *path,
int *backingFormat)
{
virStorageSourcePtr ret = NULL;
+int dummy;
+
+if (!backingFormat)
+backingFormat = &dummy;
if (!(ret = virStorageFileMetadataNew(path, format)))
return NULL;
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 744a6ba..528cfad 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -301,8 +301,7 @@ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const
char *path,
size_t len,
int format,
int *backingFormat)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
-ATTRIBUTE_NONNULL(5);
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int virStorageFileChainGetBroken(virStorageSourcePtr chain,
char **broken_file);
So if we need to cripple passing NULL, why do we even have
ATTRIBUTE_NONNULL? Wouldn't it be simpler if
virStorageFileGetMetadataFromBuf would accept NULL?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] util: XML: Avoid forward function declaration
On 07/08/14 17:51, Eric Blake wrote: > On 07/08/2014 09:29 AM, Peter Krempa wrote: >> Recursive functions apparently don't need them, but I originally thought >> they do. >> --- >> src/util/virxml.c | 5 - >> 1 file changed, 5 deletions(-) > > ACK. When it comes to static functions, it's only mutually recursive > function pairs that need a forward declaration of at least one of the > two functions. > Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: storage: Fix build after 25924dec0f9329d429aadae14e273602307e2214
The commit referenced above changed function arguments of
virStorageFileGetMetadataFromBuf() but didn't tweak the
ATTRIBUTE_NONNULL tied to them. This was caught by coverity as it
actually obeys them. We disabled them for GCC and thus it didn't show
up.
Additionally in commit 3ea661deeabadc3c114dfb6f662b9fd17d714a01 I passed
NULL to the backingFormat argument which was also marked as nonnull. Use
a dummy int's address when the argument isn't supplied so that the code
doesn't need to change much.
---
src/util/virstoragefile.c | 4
src/util/virstoragefile.h | 3 +--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 505b652..de0e27a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -955,6 +955,10 @@ virStorageFileGetMetadataFromBuf(const char *path,
int *backingFormat)
{
virStorageSourcePtr ret = NULL;
+int dummy;
+
+if (!backingFormat)
+backingFormat = &dummy;
if (!(ret = virStorageFileMetadataNew(path, format)))
return NULL;
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 744a6ba..528cfad 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -301,8 +301,7 @@ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const
char *path,
size_t len,
int format,
int *backingFormat)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
-ATTRIBUTE_NONNULL(5);
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int virStorageFileChainGetBroken(virStorageSourcePtr chain,
char **broken_file);
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virEventPollDispatchHandles: Honour array boundaries
On 07/09/2014 09:57 AM, Michal Privoznik wrote: > When dispatching events from the event loop, the array of registered > handles is searched to see what handles happened an event on. However, > the array is searched in weird way: the check for the array boundaries > is at the end, so we may touch the elements after the end of the > array: > > ==10434== Invalid read of size 4 > ==10434==at 0x52D06B6: virEventPollDispatchHandles (vireventpoll.c:486) > ==10434==by 0x52D10E4: virEventPollRunOnce (vireventpoll.c:660) > ==10434==by 0x52CF207: virEventRunDefaultImpl (virevent.c:308) > ==10434==by 0x1639D1: virNetServerRun (virnetserver.c:1139) > ==10434==by 0x1220DC: main (libvirtd.c:1507) > ==10434== Address 0xc11ff04 is 4 bytes after a block of size 960 alloc'd > ==10434==at 0x4C2CA5E: realloc (in > /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > ==10434==by 0x52AD378: virReallocN (viralloc.c:245) > ==10434==by 0x52AD46E: virExpandN (viralloc.c:294) > ==10434==by 0x52AD5B1: virResizeN (viralloc.c:352) > ==10434==by 0x52CF2EC: virEventPollAddHandle (vireventpoll.c:116) > ==10434==by 0x52CEF5B: virEventAddHandle (virevent.c:78) > ==10434==by 0x11F69A90: nodeStateInitialize (node_device_udev.c:1797) > ==10434==by 0x53C3C89: virStateInitialize (libvirt.c:743) > ==10434==by 0x120563: daemonRunStateInit (libvirtd.c:919) > ==10434==by 0x5317719: virThreadHelper (virthread.c:197) > ==10434==by 0x8376F39: start_thread (in /lib64/libpthread-2.17.so) > ==10434==by 0x8A7F9FC: clone (in /lib64/libc-2.17.so) > > Signed-off-by: Michal Privoznik > --- > src/util/vireventpoll.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virEventPollDispatchHandles: Honour array boundaries
When dispatching events from the event loop, the array of registered
handles is searched to see what handles happened an event on. However,
the array is searched in weird way: the check for the array boundaries
is at the end, so we may touch the elements after the end of the
array:
==10434== Invalid read of size 4
==10434==at 0x52D06B6: virEventPollDispatchHandles (vireventpoll.c:486)
==10434==by 0x52D10E4: virEventPollRunOnce (vireventpoll.c:660)
==10434==by 0x52CF207: virEventRunDefaultImpl (virevent.c:308)
==10434==by 0x1639D1: virNetServerRun (virnetserver.c:1139)
==10434==by 0x1220DC: main (libvirtd.c:1507)
==10434== Address 0xc11ff04 is 4 bytes after a block of size 960 alloc'd
==10434==at 0x4C2CA5E: realloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==10434==by 0x52AD378: virReallocN (viralloc.c:245)
==10434==by 0x52AD46E: virExpandN (viralloc.c:294)
==10434==by 0x52AD5B1: virResizeN (viralloc.c:352)
==10434==by 0x52CF2EC: virEventPollAddHandle (vireventpoll.c:116)
==10434==by 0x52CEF5B: virEventAddHandle (virevent.c:78)
==10434==by 0x11F69A90: nodeStateInitialize (node_device_udev.c:1797)
==10434==by 0x53C3C89: virStateInitialize (libvirt.c:743)
==10434==by 0x120563: daemonRunStateInit (libvirtd.c:919)
==10434==by 0x5317719: virThreadHelper (virthread.c:197)
==10434==by 0x8376F39: start_thread (in /lib64/libpthread-2.17.so)
==10434==by 0x8A7F9FC: clone (in /lib64/libc-2.17.so)
Signed-off-by: Michal Privoznik
---
src/util/vireventpoll.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index 528b24c..13f40dc 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -477,46 +477,46 @@ static int virEventPollDispatchTimeouts(void)
static int virEventPollDispatchHandles(int nfds, struct pollfd *fds)
{
size_t i, n;
VIR_DEBUG("Dispatch %d", nfds);
/* NB, use nfds not eventLoop.handlesCount, because new
* fds might be added on end of list, and they're not
* in the fds array we've got */
for (i = 0, n = 0; n < nfds && i < eventLoop.handlesCount; n++) {
-while ((eventLoop.handles[i].fd != fds[n].fd ||
-eventLoop.handles[i].events == 0) &&
- i < eventLoop.handlesCount) {
+while (i < eventLoop.handlesCount &&
+ (eventLoop.handles[i].fd != fds[n].fd ||
+eventLoop.handles[i].events == 0)) {
i++;
}
if (i == eventLoop.handlesCount)
break;
VIR_DEBUG("i=%zu w=%d", i, eventLoop.handles[i].watch);
if (eventLoop.handles[i].deleted) {
EVENT_DEBUG("Skip deleted n=%zu w=%d f=%d", i,
eventLoop.handles[i].watch, eventLoop.handles[i].fd);
continue;
}
if (fds[n].revents) {
virEventHandleCallback cb = eventLoop.handles[i].cb;
int watch = eventLoop.handles[i].watch;
void *opaque = eventLoop.handles[i].opaque;
int hEvents = virEventPollFromNativeEvents(fds[n].revents);
PROBE(EVENT_POLL_DISPATCH_HANDLE,
"watch=%d events=%d",
watch, hEvents);
virMutexUnlock(&eventLoop.lock);
(cb)(watch, fds[n].fd, hEvents, opaque);
virMutexLock(&eventLoop.lock);
}
}
return 0;
}
/* Used post dispatch to actually remove any timers that
* were previously marked as deleted. This asynchronous
* cleanup is needed to make dispatch re-entrant safe.
*/
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: cgroup: Fix build on non-cgroup platforms
On 07/09/14 07:11, Martin Kletzander wrote: > On Tue, Jul 08, 2014 at 05:40:40PM +0200, Peter Krempa wrote: >> Commit 48f44510098cead629ede9a49ea4e840a28ccca introduced a helper > > You probably meant a48f44510098cead629ede9a49ea4e840a28ccca. > I've fixed the typos pointed by Eric and also returned the "a" at the beginning of the commit. Thanks; Pushed. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
