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@xxxxxxxxxx] > Sent: Wednesday, March 20, 2013 5:24 AM > To: Yin Olivia-R63875 > Cc: libvir-list@xxxxxxxxxx > Subject: Re: [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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list