[libvirt] [PATCH 3/4] parallels: fix parallelsDomainDefineXML for existing containers

[Dmitry Guryanov]

On 120912 16:14:47, Daniel Veillard wrote:
> On Mon, Sep 10, 2012 at 07:22:44PM +0400, Dmitry Guryanov wrote:
> > Fix code, which checks what is changed in virDomainDef structure.
> > It looks slightly different for containers and VMs: containers haven't
> > boot devices, but have init path
> > 
> > Signed-off-by: Dmitry Guryanov <dguryanov at parallels.com>
> > ---
> >  src/parallels/parallels_driver.c |   42 ++++++++++++++++++++++++++++---------
> >  1 files changed, 32 insertions(+), 10 deletions(-)
> > 
> > diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
> > index ace75a6..fd6ba88 100644
> > --- a/src/parallels/parallels_driver.c
> > +++ b/src/parallels/parallels_driver.c
> > @@ -1484,24 +1484,46 @@ parallelsApplyChanges(virDomainObjPtr dom, virDomainDefPtr new)
> >          return -1;
> >      }
> >  
> > -    /* we fill only type and arch fields in parallelsLoadDomain, so
> > -     * we can check that all other paramenters are null */
> > +    /* we fill only type and arch fields in parallelsLoadDomain for
> > +     * hvm type and also init for containers, so we can check that all
> > +     * other paramenters are null and boot devices config is default */
> > +
> >      if (!STREQ_NULLABLE(old->os.type, new->os.type) ||
> >          !STREQ_NULLABLE(old->os.arch, new->os.arch) ||
> 
>   So here you implicitely allow an update where the new os.type or
>   os.arch would be NULL, I assume that any definition in the system
>   should have those set, right ?

old->os.type and old->os.arch are always non-NULL, because we set them
in parallelsLoadDomain and don't change later. So if new->os.type/arch
is NULL or not equal to old - we report error.

> 
> > -        new->os.machine != NULL || new->os.nBootDevs != 1 ||
> > -        new->os.bootDevs[0] != VIR_DOMAIN_BOOT_DISK ||
> > -        new->os.bootmenu != 0 || new->os.init != NULL ||
> > -        new->os.initargv != NULL || new->os.kernel != NULL ||
> > -        new->os.initrd != NULL || new->os.cmdline != NULL ||
> > -        new->os.root != NULL || new->os.loader != NULL ||
> > -        new->os.bootloader != NULL || new->os.bootloaderArgs != NULL ||
> > -        new->os.smbios_mode != 0 || new->os.bios.useserial != 0) {
> > +        new->os.machine != NULL || new->os.bootmenu != 0 ||
> > +        new->os.kernel != NULL || new->os.initrd != NULL ||
> > +        new->os.cmdline != NULL || new->os.root != NULL ||
> > +        new->os.loader != NULL || new->os.bootloader != NULL ||
> > +        new->os.bootloaderArgs != NULL || new->os.smbios_mode != 0 ||
> > +        new->os.bios.useserial != 0) {
> >  
> >          virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> >                         _("changing OS parameters is not supported "
> >                           "by parallels driver"));
> >          return -1;
> >      }
> > +    if (STREQ(new->os.type, "hvm")) {
> 
>   But here you cmpare to new->os.type, which could potentially be NULL
> at this point. Shouldn't that be changed to old->os.type instead ?
> 

Here we know, that new->os.type is equal to old one, and not NULL.

> > +        if (new->os.nBootDevs != 1 ||
> > +            new->os.bootDevs[0] != VIR_DOMAIN_BOOT_DISK ||
> > +            new->os.init != NULL || new->os.initargv != NULL) {
> > +
> > +            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> > +                           _("changing OS parameters is not supported "
> > +                             "by parallels driver"));
> > +            return -1;
> > +    }
> 
>    indentation error, here we are closing the second "if"
> > +    } else {
> > +        if (new->os.nBootDevs != 0 ||
> > +            !STREQ_NULLABLE(old->os.init, new->os.init) ||
> > +            (new->os.initargv != NULL && new->os.initargv[0] != NULL)) {
> > +
> > +            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> > +                           _("changing OS parameters is not supported "
> > +                             "by parallels driver"));
> > +            return -1;
> > +        }
> > +    }
> > +
> >  
> >      if (!STREQ_NULLABLE(old->emulator, new->emulator)) {
> >          virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> 
>   Postponed with the indentation fix waiting for the 4th patch to be
>   resubmitted,
> 
> Daniel
> 
> -- 
> Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
> daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
> http://veillard.com/ | virtualization library  http://libvirt.org/