[libvirt] [sandbox PATCH 00/10] Patches 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. --Main diffs compared to previous patches. Since many hypervisors, including kvm, will not even honour requested names for disk devices we link each device under /dev/disk/by-tag e.g /dev/disk/by-tag/foobar -> /dev/sda We populate disks.cfg with tag to device mapping when we build the sandbox. After that, in each init-process {Common,Qemu}, we basically read the configuration and populate the right symlinks under /dev/disk/by-tag The common functions for modifying directories are moved under Init-util. {Common,Qemu} inits are using them. Cédric Bosdonnat (2): Add gvir_sandbox_config_has_disks function qemu: use devtmpfs rather than tmpfs to auto-populate /dev Eren Yagdiran (8): Add an utility function for guessing filetype from file extension Add configuration object for disk support Add disk parameter to virt-sandbox Add disk support to the container builder Add disk support to machine builder Init-util : Common directory functions for init-common and init-qemu Common-init: Building symlink from disks.cfg Common-builder: /dev/disk/by-tag/thetag to /dev/vdN bin/virt-sandbox.c | 37 +++ libvirt-sandbox/Makefile.am | 7 +- .../libvirt-sandbox-builder-container.c | 37 ++- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 44 ++- libvirt-sandbox/libvirt-sandbox-builder.c | 73 ++++- libvirt-sandbox/libvirt-sandbox-config-disk.c | 274 +++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config-disk.h | 82 ++++++ libvirt-sandbox/libvirt-sandbox-config.c | 300 +++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config.h | 11 + libvirt-sandbox/libvirt-sandbox-init-common.c | 51 +++- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 151 ++--------- libvirt-sandbox/libvirt-sandbox-init-util.c | 58 ++++ libvirt-sandbox/libvirt-sandbox-init-util.h | 41 +++ libvirt-sandbox/libvirt-sandbox-util.c | 72 +++++ libvirt-sandbox/libvirt-sandbox-util.h | 5 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 5 + libvirt-sandbox/tests/test-config.c | 11 + po/POTFILES.in | 1 + 19 files changed, 1112 insertions(+), 149 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-init-util.c create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.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 | 72 ++++++++++++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-util.h | 5 +++ po/POTFILES.in | 1 + 4 files changed, 79 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..32d4c4e --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-util.c @@ -0,0 +1,72 @@ +/* + * 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 <errno.h> +#include <glib/gi18n.h> + +#include "libvirt-sandbox/libvirt-sandbox.h" + +#define GVIR_SANDBOX_UTIL_ERROR gvir_sandbox_util_error_quark() + +static GQuark +gvir_sandbox_util_error_quark(void) +{ + return g_quark_from_static_string("gvir-sandbox-util"); +} + +gint gvir_sandbox_util_guess_image_format(const gchar *path, + GError **error) +{ + 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, error); +} + +gint gvir_sandbox_util_disk_format_from_str(const gchar *value, + GError **error) +{ + GEnumClass *enum_class = g_type_class_ref(GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT); + GEnumValue *enum_value; + gint ret = -1; + + if (!(enum_value = g_enum_get_value_by_nick(enum_class, value))) { + g_set_error(error, GVIR_SANDBOX_UTIL_ERROR, 0, + _("Unknown disk format '%s'"), value); + goto cleanup; + } + ret = enum_value->value; + + cleanup: + g_type_class_unref(enum_class); + return ret; +} diff --git a/libvirt-sandbox/libvirt-sandbox-util.h b/libvirt-sandbox/libvirt-sandbox-util.h index ae6b74b..890ae7f 100644 --- a/libvirt-sandbox/libvirt-sandbox-util.h +++ b/libvirt-sandbox/libvirt-sandbox-util.h @@ -29,6 +29,11 @@ G_BEGIN_DECLS +gint gvir_sandbox_util_guess_image_format(const gchar *path, GError **error); + + +gint gvir_sandbox_util_disk_format_from_str(const gchar *value, GError **error); + /** * LIBVIRT_SANDBOX_CLASS_PADDING: (skip) */ diff --git a/po/POTFILES.in b/po/POTFILES.in index 653abc5..f291b98 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -9,3 +9,4 @@ libvirt-sandbox/libvirt-sandbox-console-rpc.c libvirt-sandbox/libvirt-sandbox-context.c libvirt-sandbox/libvirt-sandbox-init-common.c libvirt-sandbox/libvirt-sandbox-rpcpacket.c +libvirt-sandbox/libvirt-sandbox-util.c -- 2.1.0

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 | 274 ++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config-disk.h | 82 +++++++ libvirt-sandbox/libvirt-sandbox-config.c | 293 ++++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config.h | 10 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 4 + libvirt-sandbox/tests/test-config.c | 11 + 8 files changed, 677 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..04f1cf0 --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-config-disk.c @@ -0,0 +1,274 @@ +/* + * 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)) + + +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; +} + +/* + * 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..f3667b5 --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-config-disk.h @@ -0,0 +1,82 @@ +/* + * 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); + +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 fbc65a6..3342706 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,166 @@ 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; + GEnumClass *enum_class = g_type_class_ref(GVIR_CONFIG_TYPE_DOMAIN_DISK_TYPE); + GEnumValue *enum_value; + + 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 (!(enum_value = g_enum_get_value_by_nick(enum_class, typeStr))) { + g_type_class_unref(enum_class); + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("Unknown disk type prefix on %s"), opt); + return FALSE; + } + g_type_class_unref(enum_class); + type = enum_value->value; + + 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, error)) < 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, error)) < 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 @@ -1757,6 +1922,85 @@ 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; + GEnumClass *enum_class = g_type_class_ref(GVIR_CONFIG_TYPE_DOMAIN_DISK_TYPE); + GEnumValue *enum_value; + + 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 (!(enum_value = g_enum_get_value_by_nick(enum_class, typeStr))) { + g_set_error(error, GVIR_SANDBOX_CONFIG_ERROR, 0, + _("Unknown disk type %s in config file"), typeStr); + goto error; + } + type = enum_value->value; + + 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, error)) < 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_type_class_unref(enum_class); + 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) @@ -1973,6 +2217,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; @@ -1993,6 +2247,35 @@ 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); + GEnumClass *klass = NULL; + GEnumValue *value = NULL; + + key = g_strdup_printf("disk.%u", i); + + klass = g_type_class_ref(GVIR_CONFIG_TYPE_DOMAIN_DISK_TYPE); + value = g_enum_get_value(klass, type); + g_type_class_unref(klass); + g_key_file_set_string(file, key, "type", value->value_nick); + + 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)); + + klass = g_type_class_ref(GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT); + value = g_enum_get_value(klass, format); + g_type_class_unref(klass); + g_key_file_set_string(file, key, "format", value->value_nick); + g_free(key); +} + static void gvir_sandbox_config_save_config_mount(GVirSandboxConfigMount *config, GKeyFile *file, guint i) @@ -2174,6 +2457,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 a17dfed..bb717ed 100644 --- a/libvirt-sandbox/libvirt-sandbox.sym +++ b/libvirt-sandbox/libvirt-sandbox.sym @@ -213,4 +213,8 @@ LIBVIRT_SANDBOX_0.2.1 { LIBVIRT_SANDBOX_0.5.2 { global: gvir_sandbox_config_mount_guest_bind_get_format; + 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 dcbe5c1..3ea2017 100644 --- a/libvirt-sandbox/tests/test-config.c +++ b/libvirt-sandbox/tests/test-config.c @@ -58,6 +58,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", @@ -96,6 +104,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 Thu, Jun 25, 2015 at 01:27:22PM +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 --- libvirt-sandbox/Makefile.am | 2 + libvirt-sandbox/libvirt-sandbox-config-disk.c | 274 ++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config-disk.h | 82 +++++++ libvirt-sandbox/libvirt-sandbox-config.c | 293 ++++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-config.h | 10 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 4 + libvirt-sandbox/tests/test-config.c | 11 + 8 files changed, 677 insertions(+) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h
Main comment is to replace 'target' with 'tag' throughout. 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 :|

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 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..ac56346 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 -- 2.1.0

On Thu, Jun 25, 2015 at 01:27:23PM +0200, Eren Yagdiran wrote:
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 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..ac56346 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
Per the discussion on the previous set of patches, we must *not* expose the libvirt <disk> target device string to the end user at all, because it means the user has to know about the differences in virtualizaton technology used. The latter patches in your series support the /dev/disk/by-tag/TAGNAME symlink to a /dev/TARGET device node, so the user should only be providing the "TAGNAMNE" and not the TARGET. ie, -disk file:data=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 would end up creating a link /dev/disk/by-tag/data which symlinked to /dev/vda on KVM, or /dev/sda on LXC, or /dev/xvda on Xen (hypothetical) etc. 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 Thu, 2015-06-25 at 14:09 +0100, Daniel P. Berrange wrote:
On Thu, Jun 25, 2015 at 01:27:23PM +0200, Eren Yagdiran wrote:
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 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..ac56346 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
Per the discussion on the previous set of patches, we must *not* expose the libvirt <disk> target device string to the end user at all, because it means the user has to know about the differences in virtualizaton technology used.
The latter patches in your series support the /dev/disk/by-tag/TAGNAME symlink to a /dev/TARGET device node, so the user should only be providing the "TAGNAMNE" and not the TARGET.
ie,
-disk file:data=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2
would end up creating a link /dev/disk/by-tag/data which symlinked to /dev/vda on KVM, or /dev/sda on LXC, or /dev/xvda on Xen (hypothetical) etc.
Ooops, I forgot that one when reviewing. The /dev/disk/by-tag/XXX is implemented, the man wasn't changed accordingly. I can change that before pushing if that is the only problem in the series. -- Cedric

On Thu, Jun 25, 2015 at 03:49:04PM +0200, Cedric Bosdonnat wrote:
On Thu, 2015-06-25 at 14:09 +0100, Daniel P. Berrange wrote:
On Thu, Jun 25, 2015 at 01:27:23PM +0200, Eren Yagdiran wrote:
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 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..ac56346 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
Per the discussion on the previous set of patches, we must *not* expose the libvirt <disk> target device string to the end user at all, because it means the user has to know about the differences in virtualizaton technology used.
The latter patches in your series support the /dev/disk/by-tag/TAGNAME symlink to a /dev/TARGET device node, so the user should only be providing the "TAGNAMNE" and not the TARGET.
ie,
-disk file:data=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2
would end up creating a link /dev/disk/by-tag/data which symlinked to /dev/vda on KVM, or /dev/sda on LXC, or /dev/xvda on Xen (hypothetical) etc.
Ooops, I forgot that one when reviewing. The /dev/disk/by-tag/XXX is implemented, the man wasn't changed accordingly.
I can change that before pushing if that is the only problem in the series.
It isn't the only problem actually - the config API still names its methods 'get_disk_target' and when it creates the libvirt XML it is using this to set the <disk><target dev=..../</disk> attribute, so it goes deeper than just docs. 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 :|

Use the new disk configuration in the container builder to provide disks in lxc containers sandboxes. --- .../libvirt-sandbox-builder-container.c | 37 +++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c b/libvirt-sandbox/libvirt-sandbox-builder-container.c index 59bfee1..3318c30 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-container.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c @@ -218,14 +218,49 @@ 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; + size_t nVirtioDev = 0; if (!GVIR_SANDBOX_BUILDER_CLASS(gvir_sandbox_builder_container_parent_class)-> 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)){ + + gchar *device = g_strdup_printf("vd%c", (char)('a' + nVirtioDev++)); + 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_VIRTIO); + gvir_config_domain_disk_set_target_dev(disk,device); + 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); -- 2.1.0

On Thu, Jun 25, 2015 at 01:27:24PM +0200, Eren Yagdiran wrote:
Use the new disk configuration in the container builder to provide disks in lxc containers sandboxes. --- .../libvirt-sandbox-builder-container.c | 37 +++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c b/libvirt-sandbox/libvirt-sandbox-builder-container.c index 59bfee1..3318c30 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-container.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c @@ -218,14 +218,49 @@ 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; + size_t nVirtioDev = 0;
if (!GVIR_SANDBOX_BUILDER_CLASS(gvir_sandbox_builder_container_parent_class)-> 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)){ + + gchar *device = g_strdup_printf("vd%c", (char)('a' + nVirtioDev++));
Disk names are utterly arbitrary in LXC, but I think I'd suggest using 'sda' rather than 'vda', since the latter suggests use of virtio, where as sda is a generic naming scheme.
+ 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");
We can probably just leave out nbd and let libvirt "do the right thing" to pick the driver based on the declared image format. ie there's no point in forcing nbd when the format is raw.
+ 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_VIRTIO);
Don't set bus at all for containers - its irrelevant and (currently) ignored. We don't want suprises if libvirt later checks the bus and rejects it for LXC.
+ gvir_config_domain_disk_set_target_dev(disk,device); + 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); +
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 :|

Use the new disk configuration in the container builder to provide disks in qemu sandboxes. The disks are virtio devices, but those shouldn't be known by the user. --- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 38 ++++++++++++++++++++--- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c index 5e6bf72..4148d4b 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c @@ -174,7 +174,8 @@ static gchar *gvir_sandbox_builder_machine_mkinitrd(GVirSandboxConfig *config, if (gvir_sandbox_config_has_networks(config)) gvir_sandbox_config_initrd_add_module(initrd, "virtio_net.ko"); if (gvir_sandbox_config_has_mounts_with_type(config, - GVIR_SANDBOX_TYPE_CONFIG_MOUNT_HOST_IMAGE)) + GVIR_SANDBOX_TYPE_CONFIG_MOUNT_HOST_IMAGE) || + gvir_sandbox_config_has_disks(config)) gvir_sandbox_config_initrd_add_module(initrd, "virtio_blk.ko"); gvir_sandbox_config_initrd_add_module(initrd, "virtio_console.ko"); #if 0 @@ -503,9 +504,9 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde 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; + size_t nVirtioDev = 0; gchar *configdir = g_strdup_printf("%s/config", statedir); gboolean ret = FALSE; @@ -538,6 +539,35 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde g_object_unref(fs); + 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)){ + gchar *device = g_strdup_printf("vd%c", (char)('a' + nVirtioDev++)); + + 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_VIRTIO); + gvir_config_domain_disk_set_target_dev(disk, + device); + gvir_config_domain_disk_set_driver(disk, diskDriver); + gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE(disk)); + g_object_unref(disk); + g_free(device); + } + tmp = tmp->next; + } + + g_list_foreach(disks, (GFunc)g_object_unref, NULL); + g_list_free(disks); + tmp = mounts = gvir_sandbox_config_get_mounts(config); while (tmp) { GVirSandboxConfigMount *mconfig = GVIR_SANDBOX_CONFIG_MOUNT(tmp->data); @@ -563,7 +593,7 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde GVirSandboxConfigMountFile *mfile = GVIR_SANDBOX_CONFIG_MOUNT_FILE(mconfig); GVirSandboxConfigMountHostImage *mimage = GVIR_SANDBOX_CONFIG_MOUNT_HOST_IMAGE(mconfig); GVirConfigDomainDiskFormat format; - gchar *target = g_strdup_printf("vd%c", (char)('a' + nHostImage++)); + gchar *target = g_strdup_printf("vd%c", (char)('a' + nVirtioDev++)); disk = gvir_config_domain_disk_new(); gvir_config_domain_disk_set_type(disk, GVIR_CONFIG_DOMAIN_DISK_FILE); -- 2.1.0

From: Cédric Bosdonnat <cbosdonnat@suse.com> Add helper function to check if a config contains disk devices. --- libvirt-sandbox/libvirt-sandbox-config.c | 7 +++++++ libvirt-sandbox/libvirt-sandbox-config.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 1 + 3 files changed, 9 insertions(+) diff --git a/libvirt-sandbox/libvirt-sandbox-config.c b/libvirt-sandbox/libvirt-sandbox-config.c index 3342706..f800cf9 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.c +++ b/libvirt-sandbox/libvirt-sandbox-config.c @@ -1305,6 +1305,13 @@ gboolean gvir_sandbox_config_add_disk_opts(GVirSandboxConfig *config, } +gboolean gvir_sandbox_config_has_disks(GVirSandboxConfig *config) +{ + GVirSandboxConfigPrivate *priv = config->priv; + return priv->disks != NULL; +} + + /** * gvir_sandbox_config_add_mount: * @config: (transfer none): the sandbox config diff --git a/libvirt-sandbox/libvirt-sandbox-config.h b/libvirt-sandbox/libvirt-sandbox-config.h index deaea68..ebbebf2 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.h +++ b/libvirt-sandbox/libvirt-sandbox-config.h @@ -131,6 +131,7 @@ gboolean gvir_sandbox_config_add_disk_strv(GVirSandboxConfig *config, gboolean gvir_sandbox_config_add_disk_opts(GVirSandboxConfig *config, const char *disk, GError **error); +gboolean gvir_sandbox_config_has_disks(GVirSandboxConfig *config); void gvir_sandbox_config_add_mount(GVirSandboxConfig *config, GVirSandboxConfigMount *mnt); diff --git a/libvirt-sandbox/libvirt-sandbox.sym b/libvirt-sandbox/libvirt-sandbox.sym index bb717ed..e5f8660 100644 --- a/libvirt-sandbox/libvirt-sandbox.sym +++ b/libvirt-sandbox/libvirt-sandbox.sym @@ -217,4 +217,5 @@ LIBVIRT_SANDBOX_0.5.2 { gvir_sandbox_config_add_disk_strv; gvir_sandbox_config_add_disk_opts; gvir_sandbox_config_disk_get_type; + gvir_sandbox_config_has_disks; } LIBVIRT_SANDBOX_0.2.1; -- 2.1.0

From: Cédric Bosdonnat <cbosdonnat@suse.com> When using devtmpfs we don't need to care about the device nodes creation: it's less risk to forget some. It also eases the creation of the devices in the init-qemu. --- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 94 +---------------------------- 1 file changed, 1 insertion(+), 93 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 750c9de..33cebed 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -53,7 +53,6 @@ static int has_command_arg(const char *name, static int debug = 0; static char line[1024]; -static char line2[1024]; static void exit_poweroff(void) __attribute__((noreturn)); @@ -173,50 +172,6 @@ mount_9pfs(const char *src, const char *dst, int mode, int readonly) } } -static int virtioblk_major = 0; -static void -create_virtioblk_device(const char *dev) -{ - int minor; - - if (virtioblk_major == 0) { - FILE *fp = fopen("/proc/devices", "r"); - if (!fp) { - fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot read /proc/devices: %s\n", - __func__, strerror(errno)); - exit_poweroff(); - } - while (fgets(line2, sizeof line2, fp)) { - if (strstr(line2, "virtblk")) { - char *end; - long l = strtol(line2, &end, 10); - if (line2 == end) { - fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot extract device major from '%s'\n", - __func__, line2); - fclose(fp); - exit_poweroff(); - } - virtioblk_major = l; - break; - } - } - fclose(fp); - - if (virtioblk_major == 0) { - fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot find virtioblk device major in /proc/devices\n", - __func__); - exit_poweroff(); - } - } - - minor = (dev[strlen(dev)-1] - 'a') * 16; - - if (mknod(dev, S_IFBLK |0700, makedev(virtioblk_major, minor)) < 0) { - fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot make dev '%s' '%llu': %s\n", - __func__, dev, makedev(virtioblk_major, minor), strerror(errno)); - exit_poweroff(); - } -} int main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) @@ -283,59 +238,14 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) } /* Main special filesystems */ - mount_other("/dev", "tmpfs", 0755); + mount_other("/dev", "devtmpfs", 0755); mount_other_opts("/dev/pts", "devpts", "gid=5,mode=620,ptmxmode=000", 0755); mount_other("/sys", "sysfs", 0755); mount_other("/proc", "proc", 0755); //mount_other("/selinux", "selinuxfs", 0755); mount_other("/dev/shm", "tmpfs", 01777); - if (mkdir("/dev/input", 0777) < 0) { - fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot make directory /dev/input: %s\n", - __func__, strerror(errno)); - exit_poweroff(); - } - -#define MKNOD(file, mode, dev) \ - do { \ - if (mknod(file, mode, dev) < 0) { \ - fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot make dev %s %llu: %s\n", \ - __func__, file, (unsigned long long)dev, strerror(errno)); \ - exit_poweroff(); \ - } \ - } while (0) - - umask(0000); - MKNOD("/dev/null", S_IFCHR |0666, makedev(1, 3)); - MKNOD("/dev/zero", S_IFCHR |0666, makedev(1, 5)); - MKNOD("/dev/full", S_IFCHR |0666, makedev(1, 7)); - MKNOD("/dev/random", S_IFCHR |0666, makedev(1, 8)); - MKNOD("/dev/urandom", S_IFCHR |0666, makedev(1, 9)); - MKNOD("/dev/console", S_IFCHR |0700, makedev(5, 1)); - MKNOD("/dev/tty", S_IFCHR |0700, makedev(5, 0)); - MKNOD("/dev/tty0", S_IFCHR |0700, makedev(4, 0)); - MKNOD("/dev/tty1", S_IFCHR |0700, makedev(4, 1)); - MKNOD("/dev/tty2", S_IFCHR |0700, makedev(4, 2)); - MKNOD("/dev/ttyS0", S_IFCHR |0700, makedev(4, 64)); - MKNOD("/dev/ttyS1", S_IFCHR |0700, makedev(4, 65)); - MKNOD("/dev/ttyS2", S_IFCHR |0700, makedev(4, 66)); - MKNOD("/dev/ttyS3", S_IFCHR |0700, makedev(4, 67)); - MKNOD("/dev/hvc0", S_IFCHR |0700, makedev(229, 0)); - MKNOD("/dev/hvc1", S_IFCHR |0700, makedev(229, 1)); - MKNOD("/dev/hvc2", S_IFCHR |0700, makedev(229, 2)); - MKNOD("/dev/fb", S_IFCHR |0700, makedev(29, 0)); - MKNOD("/dev/fb0", S_IFCHR |0700, makedev(29, 0)); - MKNOD("/dev/mem", S_IFCHR |0600, makedev(1, 1)); - MKNOD("/dev/rtc", S_IFCHR |0700, makedev(254, 0)); - MKNOD("/dev/rtc0", S_IFCHR |0700, makedev(254, 0)); - MKNOD("/dev/ptmx", S_IFCHR |0777, makedev(5, 2)); - MKNOD("/dev/input/event0", S_IFCHR |0700, makedev(13, 64)); - MKNOD("/dev/input/event1", S_IFCHR |0700, makedev(13, 65)); - MKNOD("/dev/input/event2", S_IFCHR |0700, makedev(13, 66)); - MKNOD("/dev/input/mice", S_IFCHR |0700, makedev(13, 63)); - MKNOD("/dev/input/mouse0", S_IFCHR |0700, makedev(13, 32)); umask(0022); - mount_9pfs("sandbox:config", SANDBOXCONFIGDIR, 0755, 1); if (debug) @@ -366,8 +276,6 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) fprintf(stderr, "libvirt-sandbox-init-qemu: %s: %s -> %s (%s, %s)\n", __func__, source, target, type, opts); - if (strncmp(source, "/dev/vd", 7) == 0) - create_virtioblk_device(source); if (strcmp(type, "") == 0) { struct stat st; -- 2.1.0

On Thu, Jun 25, 2015 at 01:27:27PM +0200, Eren Yagdiran wrote:
From: Cédric Bosdonnat <cbosdonnat@suse.com>
When using devtmpfs we don't need to care about the device nodes creation: it's less risk to forget some. It also eases the creation of the devices in the init-qemu. --- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 94 +---------------------------- 1 file changed, 1 insertion(+), 93 deletions(-)
ACK, that certainly simplifies life :-) 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 :|

Init-util provides common functions for creating directories for both Init-common and Init-qemu. Error handling needs to be done in calling function. --- libvirt-sandbox/Makefile.am | 4 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 57 +++++++++------------------- libvirt-sandbox/libvirt-sandbox-init-util.c | 58 +++++++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-init-util.h | 41 ++++++++++++++++++++ 4 files changed, 119 insertions(+), 41 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.c create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.h diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 8237c50..a7f1232 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -166,6 +166,7 @@ libvirt_sandbox_1_0_la_LDFLAGS = \ -version-info $(LIBVIRT_SANDBOX_VERSION_INFO) libvirt_sandbox_init_common_SOURCES = libvirt-sandbox-init-common.c \ + libvirt-sandbox-init-util.c \ $(SANDBOX_GENERATED_RPC_FILES) \ $(SANDBOX_RPC_FILES) \ $(NULL) @@ -216,7 +217,8 @@ libvirt_sandbox_init_lxc_LDADD = \ libvirt-sandbox-1.0.la \ $(NULL) -libvirt_sandbox_init_qemu_SOURCES = libvirt-sandbox-init-qemu.c +libvirt_sandbox_init_qemu_SOURCES = libvirt-sandbox-init-qemu.c \ + libvirt-sandbox-init-util.c libvirt_sandbox_init_qemu_CFLAGS = \ -DLIBEXECDIR="\"$(libexecdir)\"" \ -DSANDBOXCONFIGDIR="\"$(sandboxconfigdir)\"" \ diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 33cebed..aa1a092 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -43,6 +43,8 @@ #include <sys/reboot.h> #include <termios.h> +#include "libvirt-sandbox-init-util.h" + #define ATTR_UNUSED __attribute__((__unused__)) static void print_uptime (void); @@ -75,43 +77,13 @@ static void sig_child(int signum ATTR_UNUSED) } static void -mount_mkdir(const char *dir, int mode); - -static void -mount_mkparent(const char *dir, int mode) -{ - char *tmp = strrchr(dir, '/'); - if (tmp && tmp != dir) { - char *parent = strndup(dir, tmp - dir); - mount_mkdir(parent, mode); - free(parent); - } -} - -static void -mount_mkdir(const char *dir, int mode) -{ - mount_mkparent(dir, 0755); - - if (debug) - fprintf(stderr, "libvirt-sandbox-init-qemu: %s: %s (mode=%o)\n", __func__, dir, mode); - - if (mkdir(dir, mode) < 0) { - if (errno != EEXIST) { - fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot make directory %s: %s\n", - __func__, dir, strerror(errno)); - exit_poweroff(); - } - } -} - -static void mount_mkfile(const char *file, int mode) { int fd; - mount_mkparent(file, 0755); - + if (gvir_sandbox_init_util_mkparent(file, 0755, debug) < 0) + exit_poweroff(); + if (debug) fprintf(stderr, "libvirt-sandbox-init-qemu: %s: %s (mode=%o)\n", __func__, file, mode); @@ -132,8 +104,9 @@ mount_other_opts(const char *dst, const char *type, const char *opts, int mode) if (debug) fprintf(stderr, "libvirt-sandbox-init-qemu: %s: %s (type=%s opts=%s)\n", __func__, dst, type, opts); - mount_mkdir(dst, mode); - + if (gvir_sandbox_init_util_mkdir(dst, mode,debug) < 0) + exit_poweroff(); + if (mount("none", dst, type, 0, opts) < 0) { fprintf(stderr, "libvirt-sandbox-init-qemu: %s: cannot mount %s on %s (%s:%s): %s\n", __func__, type, dst, type, opts, strerror(errno)); @@ -160,8 +133,9 @@ mount_9pfs(const char *src, const char *dst, int mode, int readonly) if (debug) fprintf(stderr, "libvirt-sandbox-init-qemu: %s: %s -> %s (%d)\n", __func__, src, dst, readonly); - mount_mkdir(dst, mode); - + if (gvir_sandbox_init_util_mkdir(dst, mode, debug) < 0) + exit_poweroff(); + if (readonly) flags |= MS_RDONLY; @@ -286,15 +260,18 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) __func__, source, strerror(errno)); exit_poweroff(); } - if (S_ISDIR(st.st_mode)) - mount_mkdir(target, 755); + if (S_ISDIR(st.st_mode)){ + if (gvir_sandbox_init_util_mkdir(target, 755, debug) < 0) + exit_poweroff(); + } else mount_mkfile(target, 644); } else { if (strcmp(type, "tmpfs") == 0) flags |= MS_NOSUID | MS_NODEV; - mount_mkdir(target, 0755); + if (gvir_sandbox_init_util_mkdir(target, 0755, debug) < 0) + exit_poweroff(); } if (mount(source, target, type, flags, opts) < 0) { diff --git a/libvirt-sandbox/libvirt-sandbox-init-util.c b/libvirt-sandbox/libvirt-sandbox-init-util.c new file mode 100644 index 0000000..2660652 --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-init-util.c @@ -0,0 +1,58 @@ +/* + * libvirt-sandbox-init-util.c: libvirt sandbox init 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 <errno.h> +#include <sys/stat.h> +#include <stdlib.h> +#include <stdio.h> + +#include "libvirt-sandbox-init-util.h" + +int gvir_sandbox_init_util_mkdir(const char *dir, int mode, int debug) +{ + gvir_sandbox_init_util_mkparent(dir, 0755,debug); + + if (debug > 0) + fprintf(stderr, "libvirt-sandbox-init-util: %s: %s (mode=%o)\n", __func__, dir, mode); + + if (mkdir(dir, mode) < 0) { + if (errno != EEXIST) { + fprintf(stderr, "libvirt-sandbox-init-util: %s: cannot make directory %s: %s\n",__func__, dir, strerror(errno)); + return -1; + } + } + return 0; +} + +int gvir_sandbox_init_util_mkparent(const char *dir, int mode,int debug) +{ + char *tmp = strrchr(dir, '/'); + if (tmp && tmp != dir) { + char *parent = strndup(dir, tmp - dir); + gvir_sandbox_init_util_mkdir(parent, mode,debug); + free(parent); + } + return 0; +} + diff --git a/libvirt-sandbox/libvirt-sandbox-init-util.h b/libvirt-sandbox/libvirt-sandbox-init-util.h new file mode 100644 index 0000000..01cccf1 --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-init-util.h @@ -0,0 +1,41 @@ +/* + * libvirt-sandbox-init-util.h: libvirt sandbox init util header + * + * 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> + */ + +#ifndef __LIBVIRT_SANDBOX_INIT_UTIL_H__ +#define __LIBVIRT_SANDBOX_INIT_UTIL_H__ + +int gvir_sandbox_init_util_mkdir(const char *dir, int mode, int debug); + +int gvir_sandbox_init_util_mkparent(const char *dir, int mode, int debug); + + +#endif /* __LIBVIRT_SANDBOX_INIT_UTIL_H__ */ + +/* + * Local variables: + * c-indent-level: 4 + * c-basic-offset: 4 + * indent-tabs-mode: nil + * tab-width: 8 + * End: + */ + -- 2.1.0

On Thu, Jun 25, 2015 at 01:27:28PM +0200, Eren Yagdiran wrote:
Init-util provides common functions for creating directories for both Init-common and Init-qemu. Error handling needs to be done in calling function. --- libvirt-sandbox/Makefile.am | 4 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 57 +++++++++------------------- libvirt-sandbox/libvirt-sandbox-init-util.c | 58 +++++++++++++++++++++++++++++ libvirt-sandbox/libvirt-sandbox-init-util.h | 41 ++++++++++++++++++++ 4 files changed, 119 insertions(+), 41 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.c create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.h
Creating a shared file for these two functions is not really required. They are needed in init-qemu because that file only relies on POSIX APIs. In init-common we have the ability to use glib, so we you can just use
+int gvir_sandbox_init_util_mkdir(const char *dir, int mode, int debug);
g_mkdir_with_parents(dir)
+int gvir_sandbox_init_util_mkparent(const char *dir, int mode, int debug);
parent = g_path_get_dirname(dir) g_mkdir_with_parents(parent) g_free(parent) 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 :|

Similar to the existing mounts.cfg, the mapping between the device and the tag is passed by a new disks.cfg file. Common-init reads disks.cfg and maps the tags to corresponding devices --- libvirt-sandbox/libvirt-sandbox-init-common.c | 51 +++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c b/libvirt-sandbox/libvirt-sandbox-init-common.c index 68f96ba..f8b2ea5 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-common.c +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c @@ -46,6 +46,7 @@ #include <grp.h> #include "libvirt-sandbox-rpcpacket.h" +#include "libvirt-sandbox-init-util.h" static gboolean debug = FALSE; static gboolean verbose = FALSE; @@ -60,6 +61,51 @@ static void sig_child(int sig ATTR_UNUSED) abort(); } +static gboolean setup_disk_tags(void) { + FILE *fp; + gboolean ret = FALSE; + static char line[1024]; + if (debug) + fprintf(stderr, "libvirt-sandbox-init-common: %s: populate /dev/disk/by-tag/\n", + __func__); + fp = fopen(SANDBOXCONFIGDIR "/disks.cfg", "r"); + if (fp == NULL) { + fprintf(stderr, "libvirt-sandbox-init-common: %s: cannot open " SANDBOXCONFIGDIR "/disks.cfg: %s\n", + __func__, strerror(errno)); + + goto cleanup; + } + gvir_sandbox_init_util_mkdir("/dev/disk/by-tag", 0755, debug == TRUE ? 1 : 0); + while (fgets(line, sizeof line, fp)) { + char path[1024]; + char *tag = line; + char *device = strchr(tag, '\t'); + *device = '\0'; + device++; + char *tmp = strchr(device, '\n'); + *tmp = '\0'; + + if (sprintf(path, "/dev/disk/by-tag/%s", tag) < 0) { + fprintf(stderr, "libvirt-sandbox-init-common: %s: cannot compute disk path: %s\n", + __func__, strerror(errno)); + goto cleanup; + } + + if (debug) + fprintf(stderr, "libvirt-sandbox-init-common: %s: %s -> %s\n", + __func__, path, device); + + if (symlink(device, path) < 0) { + fprintf(stderr, "libvirt-sandbox-init-common: %s: cannot create symlink %s -> %s: %s\n", + __func__, path, device, strerror(errno)); + goto cleanup; + } + } + ret = TRUE; + cleanup: + fclose(fp); + return ret; +} static int start_shell(void) @@ -258,8 +304,6 @@ static gboolean setup_network_device(GVirSandboxConfigNetwork *config, return ret; } - - static gboolean setup_network(GVirSandboxConfig *config, GError **error) { int i = 0; @@ -1215,6 +1259,9 @@ int main(int argc, char **argv) { start_shell() < 0) exit(EXIT_FAILURE); + if (!setup_disk_tags()) + exit(EXIT_FAILURE); + if (!setup_network(config, &error)) goto error; -- 2.1.0

On Thu, Jun 25, 2015 at 01:27:29PM +0200, Eren Yagdiran wrote:
Similar to the existing mounts.cfg, the mapping between the device and the tag is passed by a new disks.cfg file. Common-init reads disks.cfg and maps the tags to corresponding devices --- libvirt-sandbox/libvirt-sandbox-init-common.c | 51 +++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c b/libvirt-sandbox/libvirt-sandbox-init-common.c index 68f96ba..f8b2ea5 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-common.c +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c @@ -46,6 +46,7 @@ #include <grp.h>
#include "libvirt-sandbox-rpcpacket.h" +#include "libvirt-sandbox-init-util.h"
static gboolean debug = FALSE; static gboolean verbose = FALSE; @@ -60,6 +61,51 @@ static void sig_child(int sig ATTR_UNUSED) abort(); }
+static gboolean setup_disk_tags(void) { + FILE *fp; + gboolean ret = FALSE; + static char line[1024]; + if (debug) + fprintf(stderr, "libvirt-sandbox-init-common: %s: populate /dev/disk/by-tag/\n", + __func__); + fp = fopen(SANDBOXCONFIGDIR "/disks.cfg", "r"); + if (fp == NULL) { + fprintf(stderr, "libvirt-sandbox-init-common: %s: cannot open " SANDBOXCONFIGDIR "/disks.cfg: %s\n", + __func__, strerror(errno)); + + goto cleanup; + } + gvir_sandbox_init_util_mkdir("/dev/disk/by-tag", 0755, debug == TRUE ? 1 : 0); + while (fgets(line, sizeof line, fp)) { + char path[1024]; + char *tag = line; + char *device = strchr(tag, '\t'); + *device = '\0'; + device++; + char *tmp = strchr(device, '\n'); + *tmp = '\0'; + + if (sprintf(path, "/dev/disk/by-tag/%s", tag) < 0) {
It is preferrable to use g_strdup_printf() from glib instead of sprintf into a fixed buffer with no overflow checks. 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 :|

Common builder counts the disks devices and populates disks.cfg according to that.Disk devices are always come first than host-based images.In builder-machine, mounts of the host-based images will be mounted later. --- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 6 +- libvirt-sandbox/libvirt-sandbox-builder.c | 73 +++++++++++++++++++++-- 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c index 4148d4b..db956cf 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c @@ -283,9 +283,10 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * error); gboolean ret = FALSE; GList *mounts = gvir_sandbox_config_get_mounts(config); + GList *disks = gvir_sandbox_config_get_disks(config); GList *tmp = NULL; size_t nHostBind = 0; - size_t nHostImage = 0; + guint nVirtioDev = g_list_length(disks); if (!fos) goto cleanup; @@ -304,7 +305,7 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * fstype = "9p"; options = g_strdup("trans=virtio,version=9p2000.u"); } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_HOST_IMAGE(mconfig)) { - source = g_strdup_printf("/dev/vd%c", (char)('a' + nHostImage++)); + source = g_strdup_printf("/dev/vd%c", (char)('a' + nVirtioDev++)); fstype = "ext4"; options = g_strdup(""); } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_GUEST_BIND(mconfig)) { @@ -351,6 +352,7 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * cleanup: g_list_foreach(mounts, (GFunc)g_object_unref, NULL); g_list_free(mounts); + g_list_free(disks); if (fos) g_object_unref(fos); if (!ret) diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c index bcad652..d83fd9f 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder.c +++ b/libvirt-sandbox/libvirt-sandbox-builder.c @@ -312,14 +312,75 @@ static gboolean gvir_sandbox_builder_construct_features(GVirSandboxBuilder *buil return TRUE; } +static gboolean gvir_sandbox_builder_construct_disk_cfg(GVirSandboxConfig *config, + const gchar *statedir, + GError **error) +{ -static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *builder G_GNUC_UNUSED, - GVirSandboxConfig *config G_GNUC_UNUSED, - const gchar *statedir G_GNUC_UNUSED, - GVirConfigDomain *domain G_GNUC_UNUSED, - GError **error G_GNUC_UNUSED) + guint nVirtioDev = 0; + gchar *dskfile = g_strdup_printf("%s/config/disks.cfg", statedir); + GFile *file = g_file_new_for_path(dskfile); + GFileOutputStream *fos = g_file_replace(file, + NULL, + FALSE, + G_FILE_CREATE_REPLACE_DESTINATION, + NULL, + error); + gboolean ret = FALSE; + GList *disks = gvir_sandbox_config_get_disks(config); + GList *tmp = NULL; + const gchar *target; + + if (!fos) + goto cleanup; + + tmp = disks; + while (tmp) { + GVirSandboxConfigDisk *mconfig = GVIR_SANDBOX_CONFIG_DISK(tmp->data); + gchar *device = g_strdup_printf("/dev/vd%c", (char)('a' + (nVirtioDev)++)); + gchar *line; + + target = gvir_sandbox_config_disk_get_target(mconfig); + + line = g_strdup_printf("%s\t%s\n", + target, device); + g_free(device); + + if (!g_output_stream_write_all(G_OUTPUT_STREAM(fos), + line, strlen(line), + NULL, NULL, error)) { + g_free(line); + goto cleanup; + } + g_free(line); + + tmp = tmp->next; + } + + if (!g_output_stream_close(G_OUTPUT_STREAM(fos), NULL, error)) + goto cleanup; + + ret = TRUE; + cleanup: + g_list_foreach(disks, (GFunc)g_object_unref, NULL); + g_list_free(disks); + if (fos) + g_object_unref(fos); + if (!ret) + g_file_delete(file, NULL, NULL); + g_object_unref(file); + g_free(dskfile); + return ret; + +} + +static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *builder, + GVirSandboxConfig *config, + const gchar *statedir, + GVirConfigDomain *domain, + GError **error) { - return TRUE; + return gvir_sandbox_builder_construct_disk_cfg(config,statedir,error); } static gboolean gvir_sandbox_builder_construct_security_selinux (GVirSandboxBuilder *builder, -- 2.1.0

On Thu, Jun 25, 2015 at 01:27:20PM +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.
--Main diffs compared to previous patches.
Since many hypervisors, including kvm, will not even honour requested names for disk devices we link each device under /dev/disk/by-tag
e.g /dev/disk/by-tag/foobar -> /dev/sda
Right, but several patches in this series still deal with allowing the user to specify the /dev/sda name it appears.
We populate disks.cfg with tag to device mapping when we build the sandbox. After that, in each init-process {Common,Qemu}, we basically read the configuration and populate the right symlinks under /dev/disk/by-tag
The common functions for modifying directories are moved under Init-util. {Common,Qemu} inits are using them.
On a general point, can you make sure to run 'make syntax-check' on every patch - there are style mistakes on many of these patches that would break the syntax check 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