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

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

 



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

Attachment: signature.asc
Description: OpenPGP digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]