4:17 p.m.
Jiri Denemark (2):
qemu: Use bootindex whenever possible
qemu: Drop emitBootindex parameter
src/qemu/qemu_command.c | 89 ++++++++++------------
.../qemuxml2argv-boot-menu-enable-bootindex.args | 23 ++++++
.../qemuxml2argv-boot-menu-enable-bootindex.xml | 28 +++++++
tests/qemuxml2argvtest.c | 2 +-
4 files changed, 91 insertions(+), 51 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.xml
--
2.9.0
4:17 p.m.
New subject: [libvirt] [PATCH 1/2] qemu: Use bootindex whenever possible
I'm not sure why our code claimed "-boot menu=on" cannot be used in
combination with per-device bootindex, but it was proved wrong about
four years ago by commit 8c952908. Let's always use bootindex when QEMU
supports it.
https://bugzilla.redhat.com/show_bug.cgi?id=1323085
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_command.c | 18 ++++++--------
.../qemuxml2argv-boot-menu-enable-bootindex.args | 23 ++++++++++++++++++
.../qemuxml2argv-boot-menu-enable-bootindex.xml | 28 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 2 +-
4 files changed, 59 insertions(+), 12 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 71e9e63..ecfc447 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6028,22 +6028,18 @@ qemuBuildBootCommandLine(virCommandPtr cmd,
/*
* We prefer using explicit bootindex=N parameters for predictable
* results even though domain XML doesn't use per device boot elements.
- * However, we can't use bootindex if boot menu was requested.
*/
- if (!def->os.nBootDevs) {
- /* def->os.nBootDevs is guaranteed to be > 0 unless per-device boot
- * configuration is used
- */
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) {
+ *emitBootindex = true;
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) {
+ if (def->os.nBootDevs == 0) {
+ /* def->os.nBootDevs is guaranteed to be > 0 unless per-device boot
+ * configuration is used
+ */
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("hypervisor lacks deviceboot feature"));
goto error;
}
- *emitBootindex = true;
- } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) &&
- (def->os.bootmenu != VIR_TRISTATE_BOOL_YES ||
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU))) {
- *emitBootindex = true;
+ *emitBootindex = false;
}
if (!*emitBootindex) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.args
b/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.args
new file mode 100644
index 0000000..6580b82
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.args
@@ -0,0 +1,23 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot menu=on \
+-usb \
+-drive file=/dev/cdrom,format=raw,if=none,media=cdrom,id=drive-ide0-1-0,\
+readonly=on \
+-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,bootindex=1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.xml
b/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.xml
new file mode 100644
index 0000000..831933e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.xml
@@ -0,0 +1,28 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='cdrom'/>
+ <bootmenu enable='yes'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='cdrom'>
+ <source dev='/dev/cdrom'/>
+ <target dev='hdc' bus='ide'/>
+ <readonly/>
+ <address type='drive' controller='0' bus='1'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a4b8bf4..a73db5e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -622,7 +622,7 @@ mymain(void)
DO_TEST("boot-multi", QEMU_CAPS_BOOT_MENU);
DO_TEST("boot-menu-enable",
QEMU_CAPS_BOOT_MENU);
- DO_TEST("boot-menu-enable",
+ DO_TEST("boot-menu-enable-bootindex",
QEMU_CAPS_BOOT_MENU,
QEMU_CAPS_BOOTINDEX);
DO_TEST("boot-menu-enable-with-timeout",
--
2.9.0
4:17 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: Drop emitBootindex parameter
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_command.c | 75 ++++++++++++++++++++++---------------------------
1 file changed, 34 insertions(+), 41 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ecfc447..3898ed7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1905,8 +1905,7 @@ qemuBuildDriveDevStr(const virDomainDef *def,
static int
qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
const virDomainDef *def,
- virQEMUCapsPtr qemuCaps,
- bool emitBootindex)
+ virQEMUCapsPtr qemuCaps)
{
size_t i;
unsigned int bootCD = 0;
@@ -1915,7 +1914,8 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
virBuffer fdc_opts = VIR_BUFFER_INITIALIZER;
char *fdc_opts_str = NULL;
- if ((virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) {
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) ||
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) {
/* bootDevs will get translated into either bootindex=N or boot=on
* depending on what qemu supports */
for (i = 0; i < def->os.nBootDevs; i++) {
@@ -1936,6 +1936,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
for (i = 0; i < def->ndisks; i++) {
char *optstr;
unsigned int bootindex = 0;
+ bool driveBoot = false;
virDomainDiskDefPtr disk = def->disks[i];
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
@@ -1948,20 +1949,28 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
return -1;
}
- switch (disk->device) {
- case VIR_DOMAIN_DISK_DEVICE_CDROM:
- bootindex = bootCD;
- bootCD = 0;
- break;
- case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
- bootindex = bootFloppy;
- bootFloppy = 0;
- break;
- case VIR_DOMAIN_DISK_DEVICE_DISK:
- case VIR_DOMAIN_DISK_DEVICE_LUN:
- bootindex = bootDisk;
- bootDisk = 0;
- break;
+ if (disk->info.bootIndex) {
+ bootindex = disk->info.bootIndex;
+ } else {
+ switch (disk->device) {
+ case VIR_DOMAIN_DISK_DEVICE_CDROM:
+ bootindex = bootCD;
+ bootCD = 0;
+ break;
+ case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+ bootindex = bootFloppy;
+ bootFloppy = 0;
+ break;
+ case VIR_DOMAIN_DISK_DEVICE_DISK:
+ case VIR_DOMAIN_DISK_DEVICE_LUN:
+ bootindex = bootDisk;
+ bootDisk = 0;
+ break;
+ }
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) {
+ driveBoot = !!bootindex;
+ bootindex = 0;
+ }
}
if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0)
@@ -1969,19 +1978,11 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
virCommandAddArg(cmd, "-drive");
- optstr = qemuBuildDriveStr(disk,
- emitBootindex ? false : !!bootindex,
- qemuCaps);
- if (!optstr)
+ if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps)))
return -1;
virCommandAddArg(cmd, optstr);
VIR_FREE(optstr);
- if (!emitBootindex)
- bootindex = 0;
- else if (disk->info.bootIndex)
- bootindex = disk->info.bootIndex;
-
if (qemuDiskBusNeedsDeviceArg(disk->bus)) {
if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
if (virAsprintf(&optstr, "drive%c=drive-%s",
@@ -6018,8 +6019,7 @@ qemuBuildPMCommandLine(virCommandPtr cmd,
static int
qemuBuildBootCommandLine(virCommandPtr cmd,
const virDomainDef *def,
- virQEMUCapsPtr qemuCaps,
- bool *emitBootindex)
+ virQEMUCapsPtr qemuCaps)
{
size_t i;
virBuffer boot_buf = VIR_BUFFER_INITIALIZER;
@@ -6029,8 +6029,9 @@ qemuBuildBootCommandLine(virCommandPtr cmd,
* We prefer using explicit bootindex=N parameters for predictable
* results even though domain XML doesn't use per device boot elements.
*/
- *emitBootindex = true;
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) {
+ char boot[VIR_DOMAIN_BOOT_LAST+1];
+
if (def->os.nBootDevs == 0) {
/* def->os.nBootDevs is guaranteed to be > 0 unless per-device boot
* configuration is used
@@ -6039,11 +6040,6 @@ qemuBuildBootCommandLine(virCommandPtr cmd,
_("hypervisor lacks deviceboot feature"));
goto error;
}
- *emitBootindex = false;
- }
-
- if (!*emitBootindex) {
- char boot[VIR_DOMAIN_BOOT_LAST+1];
for (i = 0; i < def->os.nBootDevs; i++) {
switch (def->os.bootDevs[i]) {
@@ -8018,7 +8014,6 @@ qemuBuildNetCommandLine(virCommandPtr cmd,
virQEMUCapsPtr qemuCaps,
virNetDevVPortProfileOp vmop,
bool standalone,
- bool emitBootindex,
size_t *nnicindexes,
int **nicindexes,
unsigned int *bootHostdevNet)
@@ -8030,7 +8025,7 @@ qemuBuildNetCommandLine(virCommandPtr cmd,
if (def->nnets) {
unsigned int bootNet = 0;
- if (emitBootindex) {
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) {
/* convert <boot dev='network'/> to bootindex since we
didn't emit
* -boot n
*/
@@ -9137,7 +9132,6 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
size_t i;
char uuid[VIR_UUID_STRING_BUFLEN];
virCommandPtr cmd = NULL;
- bool emitBootindex = false;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
unsigned int bootHostdevNet = 0;
@@ -9245,7 +9239,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildPMCommandLine(cmd, def, qemuCaps, monitor_json) < 0)
goto error;
- if (qemuBuildBootCommandLine(cmd, def, qemuCaps, &emitBootindex) < 0)
+ if (qemuBuildBootCommandLine(cmd, def, qemuCaps) < 0)
goto error;
if (qemuBuildGlobalControllerCommandLine(cmd, def, qemuCaps) < 0)
@@ -9257,15 +9251,14 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0)
goto error;
- if (qemuBuildDiskDriveCommandLine(cmd, def, qemuCaps, emitBootindex) < 0)
+ if (qemuBuildDiskDriveCommandLine(cmd, def, qemuCaps) < 0)
goto error;
if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0)
goto error;
if (qemuBuildNetCommandLine(cmd, driver, def, qemuCaps, vmop, standalone,
- emitBootindex, nnicindexes, nicindexes,
- &bootHostdevNet) < 0)
+ nnicindexes, nicindexes, &bootHostdevNet) < 0)
goto error;
if (qemuBuildSmartcardCommandLine(logManager, cmd, cfg, def, qemuCaps) < 0)
--
2.9.0
1:32 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Drop emitBootindex parameter
On Tue, Jun 28, 2016 at 11:17:35PM +0200, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_command.c | 75 ++++++++++++++++++++++---------------------------
1 file changed, 34 insertions(+), 41 deletions(-)
@@ -8030,7 +8025,7 @@ qemuBuildNetCommandLine(virCommandPtr cmd,
if (def->nnets) {
unsigned int bootNet = 0;
- if (emitBootindex) {
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) {
This will override net->info.bootindex.
Jan
2:20 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Drop emitBootindex parameter
On Wed, Jun 29, 2016 at 08:32:50 +0200, Ján Tomko wrote:
On Tue, Jun 28, 2016 at 11:17:35PM +0200, Jiri Denemark wrote:
>Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
>---
> src/qemu/qemu_command.c | 75 ++++++++++++++++++++++---------------------------
> 1 file changed, 34 insertions(+), 41 deletions(-)
>
>@@ -8030,7 +8025,7 @@ qemuBuildNetCommandLine(virCommandPtr cmd,
> if (def->nnets) {
> unsigned int bootNet = 0;
>
>- if (emitBootindex) {
>+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) {
This will override net->info.bootindex.
Could you spend just a tiny bit more words describing what you think
this hunk will break?
Thanks,
Jirka
2:58 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Drop emitBootindex parameter
On Wed, Jun 29, 2016 at 09:20:05AM +0200, Jiri Denemark wrote:
On Wed, Jun 29, 2016 at 08:32:50 +0200, Ján Tomko wrote:
> On Tue, Jun 28, 2016 at 11:17:35PM +0200, Jiri Denemark wrote:
> >Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
> >---
> > src/qemu/qemu_command.c | 75 ++++++++++++++++++++++---------------------------
> > 1 file changed, 34 insertions(+), 41 deletions(-)
> >
> >@@ -8030,7 +8025,7 @@ qemuBuildNetCommandLine(virCommandPtr cmd,
> > if (def->nnets) {
> > unsigned int bootNet = 0;
> >
> >- if (emitBootindex) {
> >+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) {
>
> This will override net->info.bootindex.
Could you spend just a tiny bit more words describing what you think
this hunk will break?
No ;)
Jan
2:57 a.m.
On Tue, Jun 28, 2016 at 11:17:33PM +0200, Jiri Denemark wrote:
Jiri Denemark (2):
qemu: Use bootindex whenever possible
qemu: Drop emitBootindex parameter
src/qemu/qemu_command.c | 89 ++++++++++------------
.../qemuxml2argv-boot-menu-enable-bootindex.args | 23 ++++++
.../qemuxml2argv-boot-menu-enable-bootindex.xml | 28 +++++++
tests/qemuxml2argvtest.c | 2 +-
4 files changed, 91 insertions(+), 51 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-bootindex.xml
ACK series
Jan
3046
days inactive
3047
days old
6 comments
2 participants
participants (2)
-
Jiri Denemark -
Ján Tomko