[libvirt] [PATCH] add dtb support

[Yin Olivia-R63875]

Thanks for the comments.

Best Regards,
Olivia

> -----Original Message-----
> From: cardoe at cardoe.com [mailto:cardoe at cardoe.com] On Behalf Of Doug
> Goldstein
> Sent: Wednesday, February 27, 2013 1:06 PM
> To: Yin Olivia-R63875
> Cc: libvir-list at redhat.com
> Subject: Re: [libvirt] [PATCH] add dtb support
> 
> On Tue, Feb 26, 2013 at 9:38 PM, Olivia Yin <hong-hua.yin at freescale.com>
> wrote:
> > Signed-off-by: Olivia Yin <hong-hua.yin at freescale.com>
> > ---
> >  src/conf/domain_conf.c  |    4 ++++
> >  src/conf/domain_conf.h  |    1 +
> >  src/qemu/qemu_command.c |    6 ++++++
> >  3 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index
> > 0c75838..07ad6b9 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -1771,6 +1771,7 @@ void virDomainDefFree(virDomainDefPtr def)
> >      VIR_FREE(def->os.kernel);
> >      VIR_FREE(def->os.initrd);
> >      VIR_FREE(def->os.cmdline);
> > +    VIR_FREE(def->os.dtb);
> >      VIR_FREE(def->os.root);
> >      VIR_FREE(def->os.loader);
> >      VIR_FREE(def->os.bootloader);
> > @@ -9993,6 +9994,7 @@ virDomainDefParseXML(virCapsPtr caps,
> >          def->os.kernel = virXPathString("string(./os/kernel[1])", ctxt);
> >          def->os.initrd = virXPathString("string(./os/initrd[1])", ctxt);
> >          def->os.cmdline = virXPathString("string(./os/cmdline[1])",
> > ctxt);
> > +        def->os.dtb = virXPathString("string(./os/dtb[1])", ctxt);
> >          def->os.root = virXPathString("string(./os/root[1])", ctxt);
> >          def->os.loader = virXPathString("string(./os/loader[1])", ctxt);
> >      }
> > @@ -14506,6 +14508,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> >                            def->os.initrd);
> >      virBufferEscapeString(buf, "    <cmdline>%s</cmdline>\n",
> >                            def->os.cmdline);
> > +    virBufferEscapeString(buf, "    <dtb>%s</dtb>\n",
> > +                          def->os.dtb);
> >      virBufferEscapeString(buf, "    <root>%s</root>\n",
> >                            def->os.root);
> >
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index
> > 4ffa4aa..892640f 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -1545,6 +1545,7 @@ struct _virDomainOSDef {
> >      char *kernel;
> >      char *initrd;
> >      char *cmdline;
> > +    char *dtb;
> >      char *root;
> >      char *loader;
> >      char *bootloader;
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index
> > dee493f..0c68778 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -5628,6 +5628,8 @@ qemuBuildCommandLine(virConnectPtr conn,
> >              virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL);
> >          if (def->os.cmdline)
> >              virCommandAddArgList(cmd, "-append", def->os.cmdline,
> > NULL);
> > +        if (def->os.dtb)
> > +            virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL);
> >      } else {
> >          virCommandAddArgList(cmd, "-bootloader", def->os.bootloader,
> NULL);
> >      }
> > @@ -8794,6 +8796,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr
> qemuCaps,
> >              WANT_VALUE();
> >              if (!(def->os.cmdline = strdup(val)))
> >                  goto no_memory;
> > +        } else if (STREQ(arg, "-dtb")) {
> > +            WANT_VALUE();
> > +            if (!(def->os.dtb = strdup(val)))
> > +                goto no_memory;
> >          } else if (STREQ(arg, "-boot")) {
> >              const char *token = NULL;
> >              WANT_VALUE();
> > --
> > 1.6.4
> 
> Thanks for the contribution. Just a little feedback to hopefully help
> improve the contribution so we can include it.
> * You will want to look at adding some docs (look at
> docs/formatdomain.html.in) and include the version where this was supported
> (1.0.4).
> * You will want to edit the schema for domains. (look at
> docs/schemas/domain.rng)
> * You will want to include at least one test, qemuxml2argvtest,
> specifically. Peek at tests/qemuxml2argvdata and generate another case with
> the correct data.
> * Lastly the change assumes that -dtb always exists, which it likely does
> not for older versions. We detect the capabilities of qemu via
> src/qemu/qemu_capabilities.c and you'll probably want to add a capability
> bit to support this.
> 
> Hope that helps.
> --
> Doug Goldstein