Re: [libvirt] [PATCH v3 1/4] QEMU: add -dtb option support

2013-03-12 Thread Yin Olivia-R63875 
Hi Daniel,

Thanks. I've submit v4 version of these patches.

Best Regards,
Olivia

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Thursday, February 28, 2013 1:08 AM
> To: Yin Olivia-R63875
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH v3 1/4] QEMU: add -dtb option support
> 
> On Wed, Feb 27, 2013 at 04:28:37PM +0800, Olivia Yin wrote:
> > Signed-off-by: Olivia Yin 
> > ---
> >  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(-)
> 
> The split of code across your patches is sub-optimal. We prefer to keep
> changes to the XML parser/formatter separate from changes to drivers.
> 
> Also the code to test the QEMU driver should be in the same patch as the
> patch which changes the command line parser.
> 
> So what you really want is 2 patches. One with domain_conf.{c,h} changes,
> RNG schema addition & schema documentation, and the other patch with all
> the QEMU driver changes + testing
> 
> 
> 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 1/4] QEMU: add -dtb option support

2013-02-27 Thread Daniel P. Berrange 
On Wed, Feb 27, 2013 at 04:28:37PM +0800, Olivia Yin wrote:
> Signed-off-by: Olivia Yin 
> ---
>  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(-)

The split of code across your patches is sub-optimal. We prefer
to keep changes to the XML parser/formatter separate from changes
to drivers.

Also the code to test the QEMU driver should be in the same
patch as the patch which changes the command line parser.

So what you really want is 2 patches. One with domain_conf.{c,h}
changes, RNG schema addition & schema documentation, and the other
patch with all the QEMU driver changes + testing


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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 1/4] QEMU: add -dtb option support

2013-02-27 Thread Olivia Yin 
Signed-off-by: Olivia Yin 
---
 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 995cf0c..9b71066 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1785,6 +1785,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);
@@ -10047,6 +10048,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);
 }
@@ -14666,6 +14668,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
   def->os.initrd);
 virBufferEscapeString(buf, "%s\n",
   def->os.cmdline);
+virBufferEscapeString(buf, "%s\n",
+  def->os.dtb);
 virBufferEscapeString(buf, "%s\n",
   def->os.root);
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5828ae2..a35f90a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1535,6 +1535,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 1c9bfc9..fb74ec5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5732,6 +5732,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);
 }
@@ -8908,6 +8910,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


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