[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v5 2/3] qemu: add dtb option supprt



Hi Eric,

You're so kind to help squash. Thank you very much.

Exactly there's one more thing need fix is that 'ppce500v2' 
should be changed as 'ppce500' to align with qemu-1.4.0.

What should I do next step? Should I post a new revision of patchset?

Best Regards,
Olivia

> -----Original Message-----
> From: Eric Blake [mailto:eblake redhat com]
> Sent: Wednesday, March 20, 2013 5:24 AM
> To: Yin Olivia-R63875
> Cc: libvir-list redhat com
> Subject: Re: [libvirt] [PATCH v5 2/3] qemu: add dtb option supprt
> 
> On 03/13/2013 10:49 PM, Olivia Yin wrote:
> > The "dtb" option sets the filename for the device tree.
> > If without this option support, "-dtb file" will be converted into
> > <qemu:commandline> in domain XML file.
> > For example, '-dtb /media/ram/test.dtb' will be converted into
> >   <qemu:commandline>
> >     <qemu:arg value='-dtb'/>
> >     <qemu:arg value='/media/ram/test.dtb'/>
> >   </qemu:commandline>
> >
> 
> > +++ b/src/qemu/qemu_command.c
> > @@ -5984,6 +5984,8 @@ qemuBuildCommandLine(virConnectPtr conn,
> >              virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL);
> >          if (def->os.cmdline)
> >              virCommandAddArgList(cmd, "-append", def->os.cmdline,
> > NULL);
> > +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DTB) && def->os.dtb)
> > +            virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL);
> 
> This silently ignores def->os.dtb if set but qemu is too old.  Instead, we
> should error out on the unsupported combination.
> 
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
> > @@ -0,0 +1 @@
> > +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
> > +/usr/bin/qemu-system-ppc -S -M ppce500v2 -m 256 -smp 1 -nographic
> > +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c
> > +-kernel /media/ram/uImage -initrd /media/ram/ramdisk -append
> > +'root=/dev/ram rw console=ttyS0,115200' -dtb /media/ram/test.dtb -usb
> > +-net none -serial pty -parallel none
> 
> Sheesh, this line is long.  Backslash-newline is your friend.
> 
> In addition to what I'm squashing after Dan's comments, I'm adding this:
> 
> diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index
> 0b56726..8626b62 100644
> --- i/src/qemu/qemu_command.c
> +++ w/src/qemu/qemu_command.c
> @@ -6152,8 +6152,15 @@ qemuBuildCommandLine(virConnectPtr conn,
>              virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL);
>          if (def->os.cmdline)
>              virCommandAddArgList(cmd, "-append", def->os.cmdline, NULL);
> -        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DTB) && def->os.dtb)
> -            virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL);
> +        if (def->os.dtb) {
> +            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DTB)) {
> +                virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL);
> +            } else {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("dtb is not supported with this QEMU
> binary"));
> +                goto error;
> +            }
> +        }
>      } else {
>          virCommandAddArgList(cmd, "-bootloader", def->os.bootloader, NULL);
>      }
> diff --git i/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
> w/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
> index a66ac51..93e8f9c 100644
> --- i/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
> +++ w/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
> @@ -1 +1,6 @@
> -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-
> system-ppc -S -M ppce500v2 -m 256 -smp 1 -nographic -monitor
> unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -kernel
> /media/ram/uImage -initrd /media/ram/ramdisk -append 'root=/dev/ram rw
> console=ttyS0,115200' -dtb /media/ram/test.dtb -usb -net none -serial pty -
> parallel none
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
> +/usr/bin/qemu-system-ppc -S -M ppce500v2 -m 256 -smp 1 -nographic \
> +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
> +-kernel /media/ram/uImage -initrd /media/ram/ramdisk \ -append
> +'root=/dev/ram rw console=ttyS0,115200' -dtb /media/ram/test.dtb \ -usb
> +-net none -serial pty -parallel none
> 
> 
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]