[libvirt] [PATCH 3/4] parallels: add disks correctly

[Dmitry Guryanov]

On Monday 19 May 2014 13:06:21 Daniel P. Berrange wrote:
> On Thu, May 08, 2014 at 11:37:09AM +0100, Daniel P. Berrange wrote:
> > On Wed, May 07, 2014 at 10:04:08PM +0400, Dmitry Guryanov wrote:
> > > Disks support in this driver was implemented with an assumption,
> > > that disk images can't be created by hand, without VM. So
> > > complex storage driver was implemented with workaround.
> > > 
> > > This is not true, we can create new disks using ploop tool.
> > > So the first step to reimplement disks support in parallels
> > > driver is to do not use information from the storage driver,
> > > until we will implement VIR_STORAGE_TYPE_VOLUME disks.
> > > 
> > > So after this patch disks can be added in the same way as
> > > in any other driver: you create a disk image and then add
> > > an entry to the XML definition of the domain with path to that
> > > image file, for example:
> > > 
> > > <disk type='file' device='disk'>
> > > 
> > >   <driver type='ploop'/>
> > >   <source file='/storage/harddisk1.hdd'/>
> > >   <target dev='sda' bus='sata'/>
> > >   <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> > > 
> > > </disk>
> > > 
> > > This patch makes parallels storage driver useless, but I'll fix it
> > > later. Now you can create an image by hand, using ploop tool,
> > > and then add it to some domain.
> > > 
> > > Signed-off-by: Dmitry Guryanov <dguryanov at parallels.com>
> > > ---
> > > 
> > >  src/parallels/parallels_driver.c | 83
> > >  ++++++++++++++-------------------------- 1 file changed, 28
> > >  insertions(+), 55 deletions(-)
> > > 
> > > diff --git a/src/parallels/parallels_driver.c
> > > b/src/parallels/parallels_driver.c index b2de12f..67b28c4 100644
> > > --- a/src/parallels/parallels_driver.c
> > > +++ b/src/parallels/parallels_driver.c
> > > @@ -1605,17 +1605,34 @@ parallelsApplyVideoParams(parallelsDomObjPtr
> > > pdom,
> > > 
> > >      return 0;
> > >  
> > >  }
> > > 
> > > -static int parallelsAddHddByVolume(parallelsDomObjPtr pdom,
> > > -                                   virDomainDiskDefPtr disk,
> > > -                                   virStoragePoolObjPtr pool,
> > > -                                   virStorageVolDefPtr voldef)
> > > +static int parallelsAddHdd(parallelsDomObjPtr pdom,
> > > +                           virDomainDiskDefPtr disk)
> > > 
> > >  {
> > >  
> > >      int ret = -1;
> > > 
> > > +    const char *src = virDomainDiskGetSource(disk);
> > > +    int type = virDomainDiskGetType(disk);
> > > 
> > >      const char *strbus;
> > >      
> > >      virCommandPtr cmd = virCommandNewArgList(PRLCTL, "set", pdom->uuid,
> > >      
> > >                                               "--device-add", "hdd",
> > >                                               NULL);
> > > 
> > > -    virCommandAddArgFormat(cmd, "--size=%lluM", voldef->target.capacity
> > > >> 20); +
> > > +    if (type == VIR_STORAGE_TYPE_FILE) {
> > > +        int format = virDomainDiskGetFormat(disk);
> > > +
> > > +        if (format != VIR_STORAGE_FILE_PLOOP)
> > > +            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> > > +                           _("Invalid disk format: %d"), type);
> > 
> > Missing a 'goto cleanup' after reporting the error, so that
> > execution returns
> 
> Since this was the only bug in the series, I've gone ahead and fixed it
> myself and pushed all four patches to git.

Thanks, Daniel, I thought you have objections against first patch, so was 
waiting for your answer :)

> 
> Regards,
> Daniel

-- 
Dmitry Guryanov