[libvirt] [sandbox PATCH 03/10] Add disk parameter to virt-sandbox

[Daniel P. Berrange]

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