[libvirt] [sandbox PATCH 0/3] Disk support for libvirt-sandbox

Hello, These patches provide disk support for libvirt-sandbox. Implemented '--disk' parameter will be useful when integrating Docker image support for libvirt-sandbox. Currently, we only support qcow2 file format and fallback is set to RAW type. Eren Yagdiran (3): Add an utility function for guessing filetype from file extension Add configuration object for disk support Add disk parameter to virt-sandbox bin/virt-sandbox.c | 37 +++ libvirt-sandbox/Makefile.am | 3 + .../libvirt-sandbox-builder-container.c | 36 ++- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 31 ++- libvirt-sandbox/libvirt-sandbox-config-disk.c | 300 +++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config-disk.h | 86 ++++++ libvirt-sandbox/libvirt-sandbox-config.c | 275 +++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config.h | 10 + libvirt-sandbox/libvirt-sandbox-util.c | 79 ++++++ libvirt-sandbox/libvirt-sandbox-util.h | 6 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 8 + libvirt-sandbox/tests/test-config.c | 11 + 13 files changed, 881 insertions(+), 2 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c -- 2.1.0

Consider the file name extension as the image type, except for .img that are usually RAW images --- libvirt-sandbox/Makefile.am | 1 + libvirt-sandbox/libvirt-sandbox-util.c | 79 ++++++++++++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-util.h | 6 +++ 3 files changed, 86 insertions(+) create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 96302cb..6917f04 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -84,6 +84,7 @@ SANDBOX_HEADER_FILES = \ $(NULL) SANDBOX_SOURCE_FILES = \ libvirt-sandbox-main.c \ + libvirt-sandbox-util.c \ libvirt-sandbox-config.c \ libvirt-sandbox-config-network.c \ libvirt-sandbox-config-network-address.c \ diff --git a/libvirt-sandbox/libvirt-sandbox-util.c b/libvirt-sandbox/libvirt-sandbox-util.c new file mode 100644 index 0000000..0ab4fac --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-util.c @@ -0,0 +1,79 @@ +/* + * libvirt-sandbox-util.c: libvirt sandbox util functions + * + * Copyright (C) 2015 Universitat Polit��cnica de Catalunya. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: Eren Yagdiran <erenyagdiran@gmail.com> + */ + +#include <config.h> +#include <string.h> + +#include "libvirt-sandbox/libvirt-sandbox.h" + +/* This array contains string values for GVirConfigDomainDiskFormat, + * order is important.*/ +static const gchar *FORMATS_STRINGS[] = { + "raw", + "dir", + "bochs", + "cloop", + "cow", + "dmg", + "iso", + "qcow", + "qcow2", + "qed", + "vmdk", + "vpc", + "fat", + "vhd", + NULL +}; + +gint gvir_sandbox_util_guess_image_format(const gchar *path){ + + gchar *tmp; + + if ((tmp = strchr(path, '.')) == NULL) { + return -1; + } + tmp = tmp + 1; + + if (strcmp(tmp,"img")==0){ + return GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW; + } + + return gvir_sandbox_util_disk_format_from_str(tmp); +} + +gint gvir_sandbox_util_disk_format_from_str(const gchar *value) +{ + gint i = 0; + + while (FORMATS_STRINGS[i] != NULL) { + if (strcmp(FORMATS_STRINGS[i], value) == 0) + return i; + i++; + } + return -1; +} + +const gchar *gvir_sandbox_util_disk_format_to_str(GVirConfigDomainDiskFormat format) +{ + return FORMATS_STRINGS[format]; +} diff --git a/libvirt-sandbox/libvirt-sandbox-util.h b/libvirt-sandbox/libvirt-sandbox-util.h index ae6b74b..fbd3785 100644 --- a/libvirt-sandbox/libvirt-sandbox-util.h +++ b/libvirt-sandbox/libvirt-sandbox-util.h @@ -29,6 +29,12 @@ G_BEGIN_DECLS +gint gvir_sandbox_util_guess_image_format(const gchar *path); + +const gchar *gvir_sandbox_util_disk_format_to_str(GVirConfigDomainDiskFormat format); + +gint gvir_sandbox_util_disk_format_from_str(const gchar *value); + /** * LIBVIRT_SANDBOX_CLASS_PADDING: (skip) */ -- 2.1.0

On Wed, Jun 10, 2015 at 01:40:08PM +0200, Eren Yagdiran wrote:
Consider the file name extension as the image type, except for .img that are usually RAW images --- libvirt-sandbox/Makefile.am | 1 + libvirt-sandbox/libvirt-sandbox-util.c | 79 ++++++++++++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-util.h | 6 +++ 3 files changed, 86 insertions(+) create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c
diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 96302cb..6917f04 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -84,6 +84,7 @@ SANDBOX_HEADER_FILES = \ $(NULL) SANDBOX_SOURCE_FILES = \ libvirt-sandbox-main.c \ + libvirt-sandbox-util.c \ libvirt-sandbox-config.c \ libvirt-sandbox-config-network.c \ libvirt-sandbox-config-network-address.c \ diff --git a/libvirt-sandbox/libvirt-sandbox-util.c b/libvirt-sandbox/libvirt-sandbox-util.c new file mode 100644 index 0000000..0ab4fac --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-util.c @@ -0,0 +1,79 @@ +/* + * libvirt-sandbox-util.c: libvirt sandbox util functions + * + * Copyright (C) 2015 Universitat Politècnica de Catalunya. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: Eren Yagdiran <erenyagdiran@gmail.com> + */ + +#include <config.h> +#include <string.h> + +#include "libvirt-sandbox/libvirt-sandbox.h" + +/* This array contains string values for GVirConfigDomainDiskFormat, + * order is important.*/ +static const gchar *FORMATS_STRINGS[] = { + "raw", + "dir", + "bochs", + "cloop", + "cow", + "dmg", + "iso", + "qcow", + "qcow2", + "qed", + "vmdk", + "vpc", + "fat", + "vhd", + NULL +};
I'm not convinced we actually need this lookup table. The libvirt-gconfig library defines a formal gobject enum type for GVirConfigDomainDiskFormat which lets you do value <-> string lookups / conversions. eg GEnumClass *klass = g_type_class_ref(GVIR_CONFIG_DOMAIN_DISK_FORMAT); GEnumValue *value = g_enum_get_value(klass, GVIR_CONFIG_DOMAIN_DISK_FORMAT_QCOW2) value->value_nick now contains the string 'qcow2'
+ +gint gvir_sandbox_util_guess_image_format(const gchar *path){
We ought to have a GError ** parameter here to return an error message.
+ + gchar *tmp; + + if ((tmp = strchr(path, '.')) == NULL) { + return -1; + } + tmp = tmp + 1; + + if (strcmp(tmp,"img")==0){ + return GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW; + } + + return gvir_sandbox_util_disk_format_from_str(tmp); +} + +gint gvir_sandbox_util_disk_format_from_str(const gchar *value)
Same here with GError **
+{ + gint i = 0; + + while (FORMATS_STRINGS[i] != NULL) { + if (strcmp(FORMATS_STRINGS[i], value) == 0) + return i; + i++; + } + return -1; +} + +const gchar *gvir_sandbox_util_disk_format_to_str(GVirConfigDomainDiskFormat format) +{ + return FORMATS_STRINGS[format]; +}
This is redundant - the g_enum apis already let callers do this conversion. 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 :|

Add the config gobject, functions to store and load the new configuration fragments and test. This will allow creating sandboxes with attached disk with a parameter formatted like file:hda=/source/file.qcow2,format=qcow2 --- libvirt-sandbox/Makefile.am | 2 + libvirt-sandbox/libvirt-sandbox-config-disk.c | 300 ++++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config-disk.h | 86 ++++++++ libvirt-sandbox/libvirt-sandbox-config.c | 275 +++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config.h | 10 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 8 + libvirt-sandbox/tests/test-config.c | 11 + 8 files changed, 693 insertions(+) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 6917f04..8237c50 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -55,6 +55,7 @@ SANDBOX_HEADER_FILES = \ libvirt-sandbox-main.h \ libvirt-sandbox-util.h \ libvirt-sandbox-config.h \ + libvirt-sandbox-config-disk.h \ libvirt-sandbox-config-network.h \ libvirt-sandbox-config-network-address.h \ libvirt-sandbox-config-network-filterref-parameter.h \ @@ -86,6 +87,7 @@ SANDBOX_SOURCE_FILES = \ libvirt-sandbox-main.c \ libvirt-sandbox-util.c \ libvirt-sandbox-config.c \ + libvirt-sandbox-config-disk.c \ libvirt-sandbox-config-network.c \ libvirt-sandbox-config-network-address.c \ libvirt-sandbox-config-network-filterref.c \ diff --git a/libvirt-sandbox/libvirt-sandbox-config-disk.c b/libvirt-sandbox/libvirt-sandbox-config-disk.c new file mode 100644 index 0000000..75d7557 --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-config-disk.c @@ -0,0 +1,300 @@ +/* + * libvirt-sandbox-config-disk.c: libvirt sandbox configuration + * + * Copyright (C) 2015 Universitat Polit��cnica de Catalunya. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: Eren Yagdiran <erenyagdiran@gmail.com> + */ + +#include <config.h> +#include <string.h> + +#include "libvirt-sandbox/libvirt-sandbox.h" + +/** + * SECTION: libvirt-sandbox-config-disk + * @short_description: Disk attachment configuration details + * @include: libvirt-sandbox/libvirt-sandbox.h + * @see_aloso: #GVirSandboxConfig + * + * Provides an object to store information about a disk attachment in the sandbox + * + */ + +#define GVIR_SANDBOX_CONFIG_DISK_GET_PRIVATE(obj) \ + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_SANDBOX_TYPE_CONFIG_DISK, GVirSandboxConfigDiskPrivate)) + +/* This array contains string values for GVirConfigDomainDiskType, + * order is important.*/ +static const gchar *TYPES_STRINGS[] = { + "file", + "block", + "dir", + "network", + NULL +}; + +struct _GVirSandboxConfigDiskPrivate +{ + GVirConfigDomainDiskType type; + gchar *target; + gchar *source; + GVirConfigDomainDiskFormat format; +}; + +G_DEFINE_TYPE(GVirSandboxConfigDisk, gvir_sandbox_config_disk, G_TYPE_OBJECT); + + +enum { + PROP_0, + PROP_TYPE, + PROP_TARGET, + PROP_SOURCE, + PROP_FORMAT +}; + +enum { + LAST_SIGNAL +}; + + + +static void gvir_sandbox_config_disk_get_property(GObject *object, + guint prop_id, + GValue *value, + GParamSpec *pspec) +{ + GVirSandboxConfigDisk *config = GVIR_SANDBOX_CONFIG_DISK(object); + GVirSandboxConfigDiskPrivate *priv = config->priv; + + switch (prop_id) { + case PROP_TARGET: + g_value_set_string(value, priv->target); + break; + case PROP_SOURCE: + g_value_set_string(value, priv->source); + break; + case PROP_TYPE: + g_value_set_enum(value, priv->type); + break; + case PROP_FORMAT: + g_value_set_enum(value, priv->format); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); + } +} + + +static void gvir_sandbox_config_disk_set_property(GObject *object, + guint prop_id, + const GValue *value, + GParamSpec *pspec) +{ + GVirSandboxConfigDisk *config = GVIR_SANDBOX_CONFIG_DISK(object); + GVirSandboxConfigDiskPrivate *priv = config->priv; + + switch (prop_id) { + case PROP_TARGET: + g_free(priv->target); + priv->target = g_value_dup_string(value); + break; + case PROP_SOURCE: + g_free(priv->source); + priv->source = g_value_dup_string(value); + break; + case PROP_TYPE: + priv->type = g_value_get_enum(value); + break; + case PROP_FORMAT: + priv->format = g_value_get_enum(value); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); + } +} + + +static void gvir_sandbox_config_disk_finalize(GObject *object) +{ + GVirSandboxConfigDisk *config = GVIR_SANDBOX_CONFIG_DISK(object); + GVirSandboxConfigDiskPrivate *priv = config->priv; + + g_free(priv->target); + g_free(priv->source); + + G_OBJECT_CLASS(gvir_sandbox_config_disk_parent_class)->finalize(object); +} + + +static void gvir_sandbox_config_disk_class_init(GVirSandboxConfigDiskClass *klass) +{ + GObjectClass *object_class = G_OBJECT_CLASS(klass); + + object_class->finalize = gvir_sandbox_config_disk_finalize; + object_class->get_property = gvir_sandbox_config_disk_get_property; + object_class->set_property = gvir_sandbox_config_disk_set_property; + + g_object_class_install_property(object_class, + PROP_TARGET, + g_param_spec_string("target", + "Target", + "The sandbox target directory", + NULL, + G_PARAM_READABLE | + G_PARAM_WRITABLE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_NAME | + G_PARAM_STATIC_NICK | + G_PARAM_STATIC_BLURB)); + + g_object_class_install_property(object_class, + PROP_SOURCE, + g_param_spec_string("source", + "Source", + "The sandbox source directory", + NULL, + G_PARAM_READABLE | + G_PARAM_WRITABLE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_NAME | + G_PARAM_STATIC_NICK | + G_PARAM_STATIC_BLURB)); + + + g_object_class_install_property(object_class, + PROP_TYPE, + g_param_spec_enum("type", + "Disk type", + "The sandbox disk type", + GVIR_CONFIG_TYPE_DOMAIN_DISK_TYPE, + GVIR_CONFIG_DOMAIN_DISK_FILE, + G_PARAM_READABLE | + G_PARAM_WRITABLE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_NAME | + G_PARAM_STATIC_NICK | + G_PARAM_STATIC_BLURB)); + + g_object_class_install_property(object_class, + PROP_FORMAT, + g_param_spec_enum("format", + "Disk format", + "The sandbox disk format", + GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT, + GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW, + G_PARAM_READABLE | + G_PARAM_WRITABLE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_NAME | + G_PARAM_STATIC_NICK | + G_PARAM_STATIC_BLURB)); + + g_type_class_add_private(klass, sizeof(GVirSandboxConfigDiskPrivate)); +} + + +static void gvir_sandbox_config_disk_init(GVirSandboxConfigDisk *config) +{ + config->priv = GVIR_SANDBOX_CONFIG_DISK_GET_PRIVATE(config); +} + +/** + * gvir_sandbox_config_disk_get_disk_type: + * @config: (transfer none): the sandbox disk config + * + * Retrieves the type property for the custom disk + * + * Returns: (transfer none): the type property of disk + */ +GVirConfigDomainDiskType gvir_sandbox_config_disk_get_disk_type(GVirSandboxConfigDisk *config) +{ + GVirSandboxConfigDiskPrivate *priv = config->priv; + return priv->type; +} + + +/** + * gvir_sandbox_config_disk_get_format: + * @config: (transfer none): the sandbox disk config + * + * Retrieves the format property for the custom disk + * + * Returns: (transfer none): the format property of disk + */ +GVirConfigDomainDiskFormat gvir_sandbox_config_disk_get_format(GVirSandboxConfigDisk *config) +{ + GVirSandboxConfigDiskPrivate *priv = config->priv; + return priv->format; +} + + +/** + * gvir_sandbox_config_disk_get_target: + * @config: (transfer none): the sandbox disk config + * + * Retrieves the target property for the custom disk + * + * Returns: (transfer none): the target property path + */ +const gchar *gvir_sandbox_config_disk_get_target(GVirSandboxConfigDisk *config) +{ + GVirSandboxConfigDiskPrivate *priv = config->priv; + return priv->target; +} + + +/** + * gvir_sandbox_config_disk_get_source: + * @config: (transfer none): the sandbox disk config + * + * Retrieves the source property for the custom disk + * + * Returns: (transfer none): the source property + */ +const gchar *gvir_sandbox_config_disk_get_source(GVirSandboxConfigDisk *config) +{ + GVirSandboxConfigDiskPrivate *priv = config->priv; + return priv->source; +} + +const gchar *gvir_sandbox_config_disk_type_to_str(GVirConfigDomainDiskType type) +{ + return TYPES_STRINGS[type]; +} + +gint gvir_sandbox_config_disk_type_from_str(const gchar *value) +{ + gint i = 0; + + while (TYPES_STRINGS[i] != NULL) { + if (strcmp(TYPES_STRINGS[i], value) == 0) + return i; + i++; + } + return -1; +} + +/* + * Local variables: + * c-indent-level: 4 + * c-basic-offset: 4 + * indent-tabs-mode: nil + * tab-width: 8 + * End: + */ + diff --git a/libvirt-sandbox/libvirt-sandbox-config-disk.h b/libvirt-sandbox/libvirt-sandbox-config-disk.h new file mode 100644 index 0000000..15975bd --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-config-disk.h @@ -0,0 +1,86 @@ +/* + * libvirt-sandbox-config-diskaccess.h: libvirt sandbox configuration + * + * Copyright (C) 2015 Universitat Polit��cnica de Catalunya. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: Eren Yagdiran <erenyagdiran@gmail.com> + */ + +#if !defined(__LIBVIRT_SANDBOX_H__) && !defined(LIBVIRT_SANDBOX_BUILD) +#error "Only <libvirt-sandbox/libvirt-sandbox.h> can be included directly." +#endif + +#ifndef __LIBVIRT_SANDBOX_CONFIG_DISK_H__ +#define __LIBVIRT_SANDBOX_CONFIG_DISK_H__ + +G_BEGIN_DECLS + +#define GVIR_SANDBOX_TYPE_CONFIG_DISK (gvir_sandbox_config_disk_get_type ()) +#define GVIR_SANDBOX_CONFIG_DISK(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_SANDBOX_TYPE_CONFIG_DISK, GVirSandboxConfigDisk)) +#define GVIR_SANDBOX_CONFIG_DISK_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_SANDBOX_TYPE_CONFIG_DISK, GVirSandboxConfigDiskClass)) +#define GVIR_SANDBOX_IS_CONFIG_DISK(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_SANDBOX_TYPE_CONFIG_DISK)) +#define GVIR_SANDBOX_IS_CONFIG_DISK_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_SANDBOX_TYPE_CONFIG_DISK)) +#define GVIR_SANDBOX_CONFIG_DISK_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_SANDBOX_TYPE_CONFIG_DISK, GVirSandboxConfigDiskClass)) + +#define GVIR_SANDBOX_TYPE_CONFIG_DISK_HANDLE (gvir_sandbox_config_disk_handle_get_type ()) + +typedef struct _GVirSandboxConfigDisk GVirSandboxConfigDisk; +typedef struct _GVirSandboxConfigDiskPrivate GVirSandboxConfigDiskPrivate; +typedef struct _GVirSandboxConfigDiskClass GVirSandboxConfigDiskClass; + +struct _GVirSandboxConfigDisk +{ + GObject parent; + + GVirSandboxConfigDiskPrivate *priv; + + /* Do not add fields to this struct */ +}; + +struct _GVirSandboxConfigDiskClass +{ + GObjectClass parent_class; + + gpointer padding[LIBVIRT_SANDBOX_CLASS_PADDING]; +}; + +GType gvir_sandbox_config_disk_get_type(void); + +const gchar *gvir_sandbox_config_disk_get_target(GVirSandboxConfigDisk *config); + +const gchar *gvir_sandbox_config_disk_get_source(GVirSandboxConfigDisk *config); + +GVirConfigDomainDiskType gvir_sandbox_config_disk_get_disk_type(GVirSandboxConfigDisk *config); + +GVirConfigDomainDiskFormat gvir_sandbox_config_disk_get_format(GVirSandboxConfigDisk *config); + +const gchar *gvir_sandbox_config_disk_type_to_str(GVirConfigDomainDiskType type); + +gint gvir_sandbox_config_disk_type_from_str(const gchar *value); + +G_END_DECLS + +#endif /* __LIBVIRT_SANDBOX_CONFIG_DISK_H__ */ + +/* + * Local variables: + * c-indent-level: 4 + * c-basic-offset: 4 + * indent-tabs-mode: nil + * tab-width: 8 + * End: + */ diff --git a/libvirt-sandbox/libvirt-sandbox-config.c b/libvirt-sandbox/libvirt-sandbox-config.c index 087b5ce..6c18b53 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.c +++ b/libvirt-sandbox/libvirt-sandbox-config.c @@ -27,6 +27,7 @@ #include <glib/gi18n.h> #include "libvirt-sandbox/libvirt-sandbox.h" +#include "libvirt-sandbox/libvirt-sandbox-util.h" #include <errno.h> #include <selinux/selinux.h> @@ -62,6 +63,7 @@ struct _GVirSandboxConfigPrivate GList *networks; GList *mounts; + GList *disks; gchar *secLabel; gboolean secDynamic; @@ -274,6 +276,9 @@ static void gvir_sandbox_config_finalize(GObject *object) g_list_foreach(priv->networks, (GFunc)g_object_unref, NULL); g_list_free(priv->networks); + g_list_foreach(priv->disks, (GFunc)g_object_unref, NULL); + g_list_free(priv->disks); + g_free(priv->name); g_free(priv->uuid); @@ -1141,6 +1146,161 @@ gboolean gvir_sandbox_config_has_networks(GVirSandboxConfig *config) /** + * gvir_sandbox_config_add_disk: + * @config: (transfer none): the sandbox config + * @dsk: (transfer none): the disk configuration + * + * Adds a new custom disk to the sandbox + * + */ +void gvir_sandbox_config_add_disk(GVirSandboxConfig *config, + GVirSandboxConfigDisk *dsk) +{ + GVirSandboxConfigPrivate *priv = config->priv; + + g_object_ref(dsk); + + priv->disks = g_list_append(priv->disks, dsk); +} + + +/** + * gvir_sandbox_config_get_disks: + * @config: (transfer none): the sandbox config + * + * Retrieves the list of custom disks in the sandbox + * + * Returns: (transfer full) (element-type GVirSandboxConfigMount): the list of disks + */ +GList *gvir_sandbox_config_get_disks(GVirSandboxConfig *config) +{ + GVirSandboxConfigPrivate *priv = config->priv; + g_list_foreach(priv->disks, (GFunc)g_object_ref, NULL); + return g_list_copy(priv->disks); +} + + +/** + * gvir_sandbox_config_add_disk_strv: + * @config: (transfer none): the sandbox config + * @disks: (transfer none)(array zero-terminated=1): the list of disks + * + * Parses @disks whose elements are in the format TYPE:TARGET=SOURCE,FORMAT=FORMAT + * creating #GVirSandboxConfigMount instances for each element. For + * example + * + * - file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 + */ +gboolean gvir_sandbox_config_add_disk_strv(GVirSandboxConfig *config, + gchar **disks, + GError **error) +{ + gsize i = 0; + while (disks && disks[i]) { + if (!gvir_sandbox_config_add_disk_opts(config, + disks[i], + error)) + return FALSE; + i++; + } + return TRUE; +} + + +/** + * gvir_sandbox_config_add_disk_opts: + * @config: (transfer none): the sandbox config + * @disk: (transfer none): the disk config + * + * Parses @disk in the format TYPE:TARGET=SOURCE,FORMAT=FORMAT + * creating #GVirSandboxConfigDisk instances for each element. For + * example + * + * - file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 + */ + +gboolean gvir_sandbox_config_add_disk_opts(GVirSandboxConfig *config, + const char *opt, + GError **error) +{ + gchar *typeStr = NULL; + gchar *formatStr = NULL; + gchar *target = NULL; + gchar *source = NULL; + GVirSandboxConfigDisk *diskConfig; + gchar *tmp; + gint type; + gint format = GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW; + + if (!(typeStr = g_strdup(opt))) + return FALSE; + + tmp = strchr(typeStr, ':'); + + if (!tmp) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("No disk type prefix on %s"), opt); + return FALSE; + } + + *tmp = '\0'; + target = tmp + 1; + if ((type = gvir_sandbox_config_disk_type_from_str(typeStr)) < 0) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("Unknown disk type prefix on %s"), opt); + return FALSE; + } + + if (type != GVIR_CONFIG_DOMAIN_DISK_FILE) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("Unimplemented disk type prefix on %s"), opt); + return FALSE; + } + + if (!(tmp = strchr(target, '='))) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("Missing disk source string on %s"), opt); + return FALSE; + } + + *tmp = '\0'; + source = tmp + 1; + + if ((tmp = strchr(source, ',')) != NULL) { + *tmp = '\0'; + formatStr = tmp + 1; + + if ((strncmp(formatStr, "format=", 7) != 0) || + (format = gvir_sandbox_util_disk_format_from_str(formatStr + 7)) < 0){ + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("Unknown disk image format: '%s'"), formatStr + 7); + return FALSE; + } + } + else { + if ((format = gvir_sandbox_util_guess_image_format(source)) < 0) { + format = GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW; + } + } + + + diskConfig = GVIR_SANDBOX_CONFIG_DISK(g_object_new(GVIR_SANDBOX_TYPE_CONFIG_DISK, + "type", type, + "target", target, + "source", source, + "format", format, + NULL)); + + gvir_sandbox_config_add_disk(config, diskConfig); + + g_object_unref(diskConfig); + g_free(typeStr); + + return TRUE; +} + + +/** * gvir_sandbox_config_add_mount: * @config: (transfer none): the sandbox config * @mnt: (transfer none): the mount configuration @@ -1705,6 +1865,81 @@ static GVirSandboxConfigMount *gvir_sandbox_config_load_config_mount(GKeyFile *f } +static GVirSandboxConfigDisk *gvir_sandbox_config_load_config_disk(GKeyFile *file, + guint i, + GError **error) +{ + GVirSandboxConfigDisk *config = NULL; + gchar *key = NULL; + gchar *target = NULL; + gchar *source = NULL; + gchar *typeStr = NULL; + gchar *formatStr = NULL; + gint type, format; + GError *e = NULL; + + key = g_strdup_printf("disk.%u", i); + if ((target = g_key_file_get_string(file, key, "target", &e)) == NULL) { + if (e->code == G_KEY_FILE_ERROR_GROUP_NOT_FOUND) { + g_error_free(e); + return NULL; + } + g_error_free(e); + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + "%s", _("Missing disk target in config file")); + goto cleanup; + } + if ((source = g_key_file_get_string(file, key, "source", NULL)) == NULL) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + "%s", _("Missing disk source in config file")); + goto cleanup; + } + if ((typeStr = g_key_file_get_string(file, key, "type", NULL)) == NULL) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + "%s", _("Missing disk type in config file")); + goto cleanup; + } + + if ((type = gvir_sandbox_config_disk_type_from_str(typeStr)) < 0) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("Unknown disk type %s in config file"), typeStr); + goto error; + } + + if ((formatStr = g_key_file_get_string(file, key, "format", NULL)) == NULL) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + "%s", _("Missing disk format in config file")); + goto cleanup; + } + + if ((format = gvir_sandbox_util_disk_format_from_str(formatStr)) < 0) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("Unknown disk format %s in config file"), formatStr); + goto error; + } + + config = GVIR_SANDBOX_CONFIG_DISK(g_object_new(GVIR_SANDBOX_TYPE_CONFIG_DISK, + "type", type, + "target", target, + "source", source, + "format", format, + NULL)); + + cleanup: + g_free(target); + g_free(source); + g_free(typeStr); + g_free(formatStr); + g_free(key); + return config; + + error: + g_object_unref(config); + config = NULL; + goto cleanup; +} + + static GVirSandboxConfigNetwork *gvir_sandbox_config_load_config_network(GKeyFile *file, guint i, GError **error) @@ -1921,6 +2156,16 @@ static gboolean gvir_sandbox_config_load_config(GVirSandboxConfig *config, } + for (i = 0 ; i < 1024 ; i++) { + GVirSandboxConfigDisk *disk; + if (!(disk = gvir_sandbox_config_load_config_disk(file, i, error)) && + *error) + goto cleanup; + if (disk) + priv->disks = g_list_append(priv->disks, disk); + } + + g_free(priv->secLabel); if ((str = g_key_file_get_string(file, "security", "label", NULL)) != NULL) priv->secLabel = str; @@ -1941,6 +2186,26 @@ static gboolean gvir_sandbox_config_load_config(GVirSandboxConfig *config, } +static void gvir_sandbox_config_save_config_disk(GVirSandboxConfigDisk *config, + GKeyFile *file, + guint i) +{ + gchar *key; + GVirConfigDomainDiskType type = gvir_sandbox_config_disk_get_disk_type(config); + GVirConfigDomainDiskFormat format = gvir_sandbox_config_disk_get_format(config); + + key = g_strdup_printf("disk.%u", i); + g_key_file_set_string(file, key, "type", + gvir_sandbox_config_disk_type_to_str(type)); + g_key_file_set_string(file, key, "source", + gvir_sandbox_config_disk_get_source(config)); + g_key_file_set_string(file, key, "target", + gvir_sandbox_config_disk_get_target(config)); + g_key_file_set_string(file, key, "format", + gvir_sandbox_util_disk_format_to_str(format)); + g_free(key); +} + static void gvir_sandbox_config_save_config_mount(GVirSandboxConfigMount *config, GKeyFile *file, guint i) @@ -2114,6 +2379,16 @@ static void gvir_sandbox_config_save_config(GVirSandboxConfig *config, } i = 0; + tmp = priv->disks; + while (tmp) { + gvir_sandbox_config_save_config_disk(tmp->data, + file, + i); + tmp = tmp->next; + i++; + } + + i = 0; tmp = priv->networks; while (tmp) { gvir_sandbox_config_save_config_network(GVIR_SANDBOX_CONFIG_NETWORK(tmp->data), diff --git a/libvirt-sandbox/libvirt-sandbox-config.h b/libvirt-sandbox/libvirt-sandbox-config.h index 1a65e3d..deaea68 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.h +++ b/libvirt-sandbox/libvirt-sandbox-config.h @@ -122,6 +122,16 @@ gboolean gvir_sandbox_config_add_network_strv(GVirSandboxConfig *config, GError **error); gboolean gvir_sandbox_config_has_networks(GVirSandboxConfig *config); +void gvir_sandbox_config_add_disk(GVirSandboxConfig *config, + GVirSandboxConfigDisk *dsk); +GList *gvir_sandbox_config_get_disks(GVirSandboxConfig *config); +gboolean gvir_sandbox_config_add_disk_strv(GVirSandboxConfig *config, + gchar **disks, + GError **error); +gboolean gvir_sandbox_config_add_disk_opts(GVirSandboxConfig *config, + const char *disk, + GError **error); + void gvir_sandbox_config_add_mount(GVirSandboxConfig *config, GVirSandboxConfigMount *mnt); GList *gvir_sandbox_config_get_mounts(GVirSandboxConfig *config); diff --git a/libvirt-sandbox/libvirt-sandbox.h b/libvirt-sandbox/libvirt-sandbox.h index adb21a1..20dc871 100644 --- a/libvirt-sandbox/libvirt-sandbox.h +++ b/libvirt-sandbox/libvirt-sandbox.h @@ -31,6 +31,7 @@ #include <libvirt-sandbox/libvirt-sandbox-main.h> #include <libvirt-sandbox/libvirt-sandbox-util.h> #include <libvirt-sandbox/libvirt-sandbox-enum-types.h> +#include <libvirt-sandbox/libvirt-sandbox-config-disk.h> #include <libvirt-sandbox/libvirt-sandbox-config-mount.h> #include <libvirt-sandbox/libvirt-sandbox-config-mount-file.h> #include <libvirt-sandbox/libvirt-sandbox-config-mount-host-bind.h> diff --git a/libvirt-sandbox/libvirt-sandbox.sym b/libvirt-sandbox/libvirt-sandbox.sym index 7afef53..33411de 100644 --- a/libvirt-sandbox/libvirt-sandbox.sym +++ b/libvirt-sandbox/libvirt-sandbox.sym @@ -209,3 +209,11 @@ LIBVIRT_SANDBOX_0.2.1 { local: *; }; + +LIBVIRT_SANDBOX_0.5.1 { + global: + gvir_sandbox_config_add_disk; + gvir_sandbox_config_add_disk_strv; + gvir_sandbox_config_add_disk_opts; + gvir_sandbox_config_disk_get_type; +} LIBVIRT_SANDBOX_0.2.1; diff --git a/libvirt-sandbox/tests/test-config.c b/libvirt-sandbox/tests/test-config.c index 1afec83..3483eb5 100644 --- a/libvirt-sandbox/tests/test-config.c +++ b/libvirt-sandbox/tests/test-config.c @@ -57,6 +57,14 @@ int main(int argc, char **argv) "host-bind:/tmp=", NULL }; + const gchar *disks[] = { + "file:hda=/tmp/img.blah,format=qcow2", + "file:hda=/tmp/img.qcow2", + "file:hda=/tmp/img.qcow2,format=raw", + "file:hda=/tmp/img.img", + "file:hda=/tmp/imq", + NULL + }; const gchar *includes[] = { "/etc/nswitch.conf", "/etc/resolve.conf", @@ -95,6 +103,9 @@ int main(int argc, char **argv) if (!gvir_sandbox_config_add_mount_strv(cfg1, (gchar**)mounts, &err)) goto cleanup; + if (!gvir_sandbox_config_add_disk_strv(cfg1, (gchar**)disks, &err)) + goto cleanup; + if (!gvir_sandbox_config_add_host_include_strv(cfg1, (gchar**)includes, &err)) goto cleanup; -- 2.1.0

On Wed, Jun 10, 2015 at 01:40:09PM +0200, Eren Yagdiran wrote:
Add the config gobject, functions to store and load the new configuration fragments and test. This will allow creating sandboxes with attached disk with a parameter formatted like file:hda=/source/file.qcow2,format=qcow2
+/** + * gvir_sandbox_config_add_disk_strv: + * @config: (transfer none): the sandbox config + * @disks: (transfer none)(array zero-terminated=1): the list of disks + * + * Parses @disks whose elements are in the format TYPE:TARGET=SOURCE,FORMAT=FORMAT + * creating #GVirSandboxConfigMount instances for each element. For + * example + * + * - file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 + */
One of the goal of the libvirt sandbox code is to insulate apps from needing to know hypervisor specific differences. The guest side disk device name is one such big difference. Many hypervisors, including kvm, will not even honour requested names - you just get whatever name the guest decides to give you. Essentially the only thing that libvirt guarantees is the disk ordering. ie if you configure two disks one with hda and one hdb, libvirt will ensure hda appears before hdb on the bus or controller. So I don't think we should include the target device name in our configuration syntax here. We should just document that disks will be added to the guest in the same order that you supply them to virt-sandbox command line in. The actual device names will vary according to the hypevisor and guest os.
+gboolean gvir_sandbox_config_add_disk_strv(GVirSandboxConfig *config, + gchar **disks, + GError **error) +{ + gsize i = 0; + while (disks && disks[i]) { + if (!gvir_sandbox_config_add_disk_opts(config, + disks[i], + error)) + return FALSE; + i++; + } + return TRUE; +}
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 :|

Hi Daniel, On Thu, 2015-06-11 at 14:47 +0100, Daniel P. Berrange wrote:
On Wed, Jun 10, 2015 at 01:40:09PM +0200, Eren Yagdiran wrote:
Add the config gobject, functions to store and load the new configuration fragments and test. This will allow creating sandboxes with attached disk with a parameter formatted like file:hda=/source/file.qcow2,format=qcow2
+/** + * gvir_sandbox_config_add_disk_strv: + * @config: (transfer none): the sandbox config + * @disks: (transfer none)(array zero-terminated=1): the list of disks + * + * Parses @disks whose elements are in the format TYPE:TARGET=SOURCE,FORMAT=FORMAT + * creating #GVirSandboxConfigMount instances for each element. For + * example + * + * - file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 + */
One of the goal of the libvirt sandbox code is to insulate apps from needing to know hypervisor specific differences. The guest side disk device name is one such big difference. Many hypervisors, including kvm, will not even honour requested names - you just get whatever name the guest decides to give you. Essentially the only thing that libvirt guarantees is the disk ordering. ie if you configure two disks one with hda and one hdb, libvirt will ensure hda appears before hdb on the bus or controller.
So I don't think we should include the target device name in our configuration syntax here. We should just document that disks will be added to the guest in the same order that you supply them to virt-sandbox command line in. The actual device names will vary according to the hypevisor and guest os.
Right. But I think we then need to ask for the bus type then, so that we know whether we'll have ide, scsi or whatever. Then we could have the option like: file:ide=/path/to/source,format=fmt And of course document that the disks will be added in the same order. Does it sound better for you? -- Cedric
+gboolean gvir_sandbox_config_add_disk_strv(GVirSandboxConfig *config, + gchar **disks, + GError **error) +{ + gsize i = 0; + while (disks && disks[i]) { + if (!gvir_sandbox_config_add_disk_opts(config, + disks[i], + error)) + return FALSE; + i++; + } + return TRUE; +}
Regards, Daniel

On Wed, Jun 17, 2015 at 09:43:23AM +0200, Cedric Bosdonnat wrote:
Hi Daniel,
On Thu, 2015-06-11 at 14:47 +0100, Daniel P. Berrange wrote:
On Wed, Jun 10, 2015 at 01:40:09PM +0200, Eren Yagdiran wrote:
Add the config gobject, functions to store and load the new configuration fragments and test. This will allow creating sandboxes with attached disk with a parameter formatted like file:hda=/source/file.qcow2,format=qcow2
+/** + * gvir_sandbox_config_add_disk_strv: + * @config: (transfer none): the sandbox config + * @disks: (transfer none)(array zero-terminated=1): the list of disks + * + * Parses @disks whose elements are in the format TYPE:TARGET=SOURCE,FORMAT=FORMAT + * creating #GVirSandboxConfigMount instances for each element. For + * example + * + * - file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 + */
One of the goal of the libvirt sandbox code is to insulate apps from needing to know hypervisor specific differences. The guest side disk device name is one such big difference. Many hypervisors, including kvm, will not even honour requested names - you just get whatever name the guest decides to give you. Essentially the only thing that libvirt guarantees is the disk ordering. ie if you configure two disks one with hda and one hdb, libvirt will ensure hda appears before hdb on the bus or controller.
So I don't think we should include the target device name in our configuration syntax here. We should just document that disks will be added to the guest in the same order that you supply them to virt-sandbox command line in. The actual device names will vary according to the hypevisor and guest os.
Right. But I think we then need to ask for the bus type then, so that we know whether we'll have ide, scsi or whatever. Then we could have the option like:
file:ide=/path/to/source,format=fmt
And of course document that the disks will be added in the same order.
Does it sound better for you?
Not really - that is still not achieving our goal that users should not need to details about the hypervisor used when configuring a sandbox. The goal is that the CLI args to virt-sandbox should be unchanged, no matter what libvirt URI is provided. Now of course in the case of virtualized disks, what appears inside the sandbox could be differently named - ideally we'd find a way to insulate users from that aspect too. In normal Linux there are various symlinks /dev/disk/by-{uuid,id,path} available which link back to /dev/[sda,vda,hda,etc]. These are created by udev, but we're not running that. I wonder if we could make use of that concept though - create /dev/disk/by-tag directory and then in the cli args we could allow a simple tag. eg file:foobar=/path/to/source,format=fmt and then populate a symlink /dev/disk/by-tag/foobar -> /dev/sda 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 :|

On Wed, 2015-06-17 at 10:35 +0100, Daniel P. Berrange wrote:
On Wed, Jun 17, 2015 at 09:43:23AM +0200, Cedric Bosdonnat wrote:
Hi Daniel,
On Thu, 2015-06-11 at 14:47 +0100, Daniel P. Berrange wrote:
On Wed, Jun 10, 2015 at 01:40:09PM +0200, Eren Yagdiran wrote:
Add the config gobject, functions to store and load the new configuration fragments and test. This will allow creating sandboxes with attached disk with a parameter formatted like file:hda=/source/file.qcow2,format=qcow2
+/** + * gvir_sandbox_config_add_disk_strv: + * @config: (transfer none): the sandbox config + * @disks: (transfer none)(array zero-terminated=1): the list of disks + * + * Parses @disks whose elements are in the format TYPE:TARGET=SOURCE,FORMAT=FORMAT + * creating #GVirSandboxConfigMount instances for each element. For + * example + * + * - file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 + */
One of the goal of the libvirt sandbox code is to insulate apps from needing to know hypervisor specific differences. The guest side disk device name is one such big difference. Many hypervisors, including kvm, will not even honour requested names - you just get whatever name the guest decides to give you. Essentially the only thing that libvirt guarantees is the disk ordering. ie if you configure two disks one with hda and one hdb, libvirt will ensure hda appears before hdb on the bus or controller.
So I don't think we should include the target device name in our configuration syntax here. We should just document that disks will be added to the guest in the same order that you supply them to virt-sandbox command line in. The actual device names will vary according to the hypevisor and guest os.
Right. But I think we then need to ask for the bus type then, so that we know whether we'll have ide, scsi or whatever. Then we could have the option like:
file:ide=/path/to/source,format=fmt
And of course document that the disks will be added in the same order.
Does it sound better for you?
Not really - that is still not achieving our goal that users should not need to details about the hypervisor used when configuring a sandbox.
The goal is that the CLI args to virt-sandbox should be unchanged, no matter what libvirt URI is provided. Now of course in the case of virtualized disks, what appears inside the sandbox could be differently named - ideally we'd find a way to insulate users from that aspect too.
In normal Linux there are various symlinks /dev/disk/by-{uuid,id,path} available which link back to /dev/[sda,vda,hda,etc]. These are created by udev, but we're not running that. I wonder if we could make use of that concept though - create /dev/disk/by-tag directory and then in the cli args we could allow a simple tag. eg
file:foobar=/path/to/source,format=fmt
and then populate a symlink /dev/disk/by-tag/foobar -> /dev/sda
I love this idea :) That would nicely solve the problem. -- Cedric

Allow users to add disk images to their sandbox. Only disk images are supported so far, but the parameter is intentionally designed for future changes. --- bin/virt-sandbox.c | 37 ++++++++++++++++++++++ .../libvirt-sandbox-builder-container.c | 36 ++++++++++++++++++++- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 31 +++++++++++++++++- 3 files changed, 102 insertions(+), 2 deletions(-) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..aa09f33 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -63,6 +63,7 @@ int main(int argc, char **argv) { GMainLoop *loop = NULL; GError *error = NULL; gchar *name = NULL; + gchar **disks = NULL; gchar **mounts = NULL; gchar **includes = NULL; gchar *includefile = NULL; @@ -92,6 +93,8 @@ int main(int argc, char **argv) { N_("name of the sandbox"), "NAME" }, { "root", 'r', 0, G_OPTION_ARG_STRING, &root, N_("root directory of the sandbox"), "DIR" }, + { "disk", ' ', 0, G_OPTION_ARG_STRING_ARRAY, &disks, + N_("add a disk in the guest"), "TYPE:TARGET=SOURCE,FORMAT=FORMAT" }, { "mount", 'm', 0, G_OPTION_ARG_STRING_ARRAY, &mounts, N_("mount a filesystem in the guest"), "TYPE:TARGET=SOURCE" }, { "include", 'i', 0, G_OPTION_ARG_STRING_ARRAY, &includes, @@ -182,6 +185,13 @@ int main(int argc, char **argv) { gvir_sandbox_config_set_username(cfg, "root"); } + if (disks && + !gvir_sandbox_config_add_disk_strv(cfg, disks, &error)) { + g_printerr(_("Unable to parse disks: %s\n"), + error && error->message ? error->message : _("Unknown failure")); + goto cleanup; + } + if (mounts && !gvir_sandbox_config_add_mount_strv(cfg, mounts, &error)) { g_printerr(_("Unable to parse mounts: %s\n"), @@ -319,6 +329,33 @@ inheriting the host's root filesystem. NB. C<DIR> must contain a matching install of the libvirt-sandbox package. This restriction may be lifted in a future version. +=item B<--disk TYPE:TARGET=SOURCE,FORMAT=FORMAT> + +Sets up a disk inside the sandbox by using B<SOURCE> with target device B<TARGET> +and type B<TYPE> and format B<FORMAT>. Example: file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 +Format is an optional parameter. + +=over 4 + +=item B<TYPE> + +Type parameter can be set to "file". + +=item B<TARGET> + +Target parameter can be set to "hda" or any other libvirt supported target disk + +=item B<SOURCE> + +Source parameter needs to point a file which must be a one of the valid domain disk formats supported by qemu. + +=item B<FORMAT> + +Format parameter must be set to the same disk format as the file passed on source parameter. +This parameter is optional and the format can be guessed from the image extension + +=back + =item B<-m TYPE:DST=SRC>, B<--mount TYPE:DST=SRC> Sets up a mount inside the sandbox at B<DST> backed by B<SRC>. The diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c b/libvirt-sandbox/libvirt-sandbox-builder-container.c index c3a58b2..9839810 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-container.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c @@ -218,7 +218,9 @@ static gboolean gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil GVirConfigDomainInterfaceNetwork *iface; GVirConfigDomainConsole *con; GVirConfigDomainChardevSourcePty *src; - GList *tmp = NULL, *mounts = NULL, *networks = NULL; + GVirConfigDomainDisk *disk; + GVirConfigDomainDiskDriver *diskDriver; + GList *tmp = NULL, *mounts = NULL, *networks = NULL, *disks = NULL; gchar *configdir = g_strdup_printf("%s/config", statedir); gboolean ret = FALSE; @@ -226,6 +228,38 @@ static gboolean gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil construct_devices(builder, config, statedir, domain, error)) goto cleanup; + + tmp = disks = gvir_sandbox_config_get_disks(config); + while(tmp){ + GVirSandboxConfigDisk *dconfig = GVIR_SANDBOX_CONFIG_DISK(tmp->data); + + if (GVIR_SANDBOX_IS_CONFIG_DISK(dconfig)){ + + disk = gvir_config_domain_disk_new(); + diskDriver = gvir_config_domain_disk_driver_new(); + gvir_config_domain_disk_set_type(disk, + gvir_sandbox_config_disk_get_disk_type(dconfig)); + gvir_config_domain_disk_driver_set_format(diskDriver, + gvir_sandbox_config_disk_get_format(dconfig)); + gvir_config_domain_disk_driver_set_name(diskDriver, "nbd"); + gvir_config_domain_disk_set_source(disk, + gvir_sandbox_config_disk_get_source(dconfig)); + gvir_config_domain_disk_set_target_bus(disk, + GVIR_CONFIG_DOMAIN_DISK_BUS_IDE); + gvir_config_domain_disk_set_target_dev(disk, + gvir_sandbox_config_disk_get_target(dconfig)); + gvir_config_domain_disk_set_driver(disk, diskDriver); + gvir_config_domain_add_device(domain, + GVIR_CONFIG_DOMAIN_DEVICE(disk)); + g_object_unref(disk); + } + tmp = tmp->next; + } + + g_list_foreach(disks, (GFunc)g_object_unref, NULL); + g_list_free(disks); + + fs = gvir_config_domain_filesys_new(); gvir_config_domain_filesys_set_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_MOUNT); gvir_config_domain_filesys_set_access_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_PASSTHROUGH); diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c index 35a5816..255c52d 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c @@ -497,12 +497,13 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde { GVirConfigDomainFilesys *fs; GVirConfigDomainDisk *disk; + GVirConfigDomainDiskDriver *diskDriver; GVirConfigDomainInterface *iface; GVirConfigDomainMemballoon *ball; GVirConfigDomainConsole *con; GVirConfigDomainSerial *ser; GVirConfigDomainChardevSourcePty *src; - GList *tmp = NULL, *mounts = NULL, *networks = NULL; + GList *tmp = NULL, *mounts = NULL, *networks = NULL, *disks = NULL; size_t nHostBind = 0; size_t nHostImage = 0; gchar *configdir = g_strdup_printf("%s/config", statedir); @@ -512,6 +513,34 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde construct_devices(builder, config, statedir, domain, error)) goto cleanup; + tmp = disks = gvir_sandbox_config_get_disks(config); + while(tmp){ + GVirSandboxConfigDisk *dconfig = GVIR_SANDBOX_CONFIG_DISK(tmp->data); + + if (GVIR_SANDBOX_IS_CONFIG_DISK(dconfig)){ + + disk = gvir_config_domain_disk_new(); + diskDriver = gvir_config_domain_disk_driver_new(); + gvir_config_domain_disk_set_type(disk, + gvir_sandbox_config_disk_get_disk_type(dconfig)); + gvir_config_domain_disk_driver_set_format(diskDriver, + gvir_sandbox_config_disk_get_format(dconfig)); + gvir_config_domain_disk_set_source(disk, gvir_sandbox_config_disk_get_source(dconfig)); + gvir_config_domain_disk_set_target_bus(disk, + GVIR_CONFIG_DOMAIN_DISK_BUS_IDE); + gvir_config_domain_disk_set_target_dev(disk, + gvir_sandbox_config_disk_get_target(dconfig)); + gvir_config_domain_disk_set_driver(disk, diskDriver); + gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE(disk)); + g_object_unref(disk); + } + tmp = tmp->next; + } + + g_list_foreach(disks, (GFunc)g_object_unref, NULL); + g_list_free(disks); + + fs = gvir_config_domain_filesys_new(); gvir_config_domain_filesys_set_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_MOUNT); gvir_config_domain_filesys_set_access_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_SQUASH); -- 2.1.0

On Wed, Jun 10, 2015 at 01:40:07PM +0200, Eren Yagdiran wrote:
Hello,
These patches provide disk support for libvirt-sandbox. Implemented '--disk' parameter will be useful when integrating Docker image support for libvirt-sandbox. Currently, we only support qcow2 file format and fallback is set to RAW type.
Can you explain a bit more why you need to be able to expose a virtual disk to the sandbox. If the stuff running inside the sandbox is not privileged, it won't even have access to the device node inside it, nor be able to mount it. This is why the sandbox code focuses on mounting everything itself before handing off the main app to run. So I'm not really clear how this feature is going to be generally useful or usable. 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 :|

On Wed, 2015-06-10 at 13:06 +0100, Daniel P. Berrange wrote:
On Wed, Jun 10, 2015 at 01:40:07PM +0200, Eren Yagdiran wrote:
Hello,
These patches provide disk support for libvirt-sandbox. Implemented '--disk' parameter will be useful when integrating Docker image support for libvirt-sandbox. Currently, we only support qcow2 file format and fallback is set to RAW type.
Can you explain a bit more why you need to be able to expose a virtual disk to the sandbox. If the stuff running inside the sandbox is not privileged, it won't even have access to the device node inside it, nor be able to mount it. This is why the sandbox code focuses on mounting everything itself before handing off the main app to run. So I'm not really clear how this feature is going to be generally useful or usable.
Before filling the qcow2 images, we need to be able to format them... would you then suggest to manually setup the nbd device, and run mkfs on it? I was thinking about having that done through virt-sandbox... -- Cedric

On Wed, Jun 10, 2015 at 03:13:15PM +0200, Cedric Bosdonnat wrote:
On Wed, 2015-06-10 at 13:06 +0100, Daniel P. Berrange wrote:
On Wed, Jun 10, 2015 at 01:40:07PM +0200, Eren Yagdiran wrote:
Hello,
These patches provide disk support for libvirt-sandbox. Implemented '--disk' parameter will be useful when integrating Docker image support for libvirt-sandbox. Currently, we only support qcow2 file format and fallback is set to RAW type.
Can you explain a bit more why you need to be able to expose a virtual disk to the sandbox. If the stuff running inside the sandbox is not privileged, it won't even have access to the device node inside it, nor be able to mount it. This is why the sandbox code focuses on mounting everything itself before handing off the main app to run. So I'm not really clear how this feature is going to be generally useful or usable.
Before filling the qcow2 images, we need to be able to format them... would you then suggest to manually setup the nbd device, and run mkfs on it? I was thinking about having that done through virt-sandbox...
Ah ha, ok, that makes some sense now. So this isn't really for the purpose of executing docker images - its for the earlier step where we actually turn the downloaded docker images into something we can run. This stage of the process would be running privileged, so it makes sense it can use a block device. 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 :|
participants (3)
-
Cedric Bosdonnat
-
Daniel P. Berrange
-
Eren Yagdiran