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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]