[libvirt] [sandbox PATCH 04/10] Add disk support to the container builder

[Daniel P. Berrange]

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 :|