On Wed, May 30, 2018 at 04:00:32PM -0400, John Ferlan wrote:
This is way too sparse.
I can't really think of what to say here that is not already mentioned in the documentation or self-explanatory in the code. But maybe only I see that as self-explanatory. I'll write something up here.
On 05/21/2018 11:00 AM, Martin Kletzander wrote:Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1469338 Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- src/qemu/qemu_command.c | 18 ++++ src/qemu/qemu_domain.c | 84 +++++++++++++++++++ .../qemuxml2argvdata/tseg-explicit-size.args | 28 +++++++ tests/qemuxml2argvdata/tseg-explicit-size.xml | 23 +++++ tests/qemuxml2argvdata/tseg-i440fx.xml | 23 +++++ tests/qemuxml2argvdata/tseg-invalid-size.xml | 23 +++++ .../tseg-old-machine-type.args | 27 ++++++ .../tseg-old-machine-type.xml | 21 +++++ tests/qemuxml2argvdata/tseg.args | 28 +++++++ tests/qemuxml2argvdata/tseg.xml | 21 +++++ tests/qemuxml2argvtest.c | 48 +++++++++++ .../qemuxml2xmloutdata/tseg-explicit-size.xml | 46 ++++++++++ .../tseg-old-machine-type.xml | 44 ++++++++++ tests/qemuxml2xmloutdata/tseg.xml | 46 ++++++++++ tests/qemuxml2xmltest.c | 25 ++++++ 15 files changed, 505 insertions(+) create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.args create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2argvdata/tseg-i440fx.xml create mode 100644 tests/qemuxml2argvdata/tseg-invalid-size.xml create mode 100644 tests/qemuxml2argvdata/tseg-old-machine-type.args create mode 100644 tests/qemuxml2argvdata/tseg-old-machine-type.xml create mode 100644 tests/qemuxml2argvdata/tseg.args create mode 100644 tests/qemuxml2argvdata/tseg.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-explicit-size.xml create mode 100644 tests/qemuxml2xmloutdata/tseg-old-machine-type.xml create mode 100644 tests/qemuxml2xmloutdata/tseg.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 328f3c0a2386..36f557676fb0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7200,6 +7200,22 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, return ret; } + +static void +qemuBuildTSEGCommandLine(virCommandPtr cmd, + const virDomainDef *def) +{ + if (!def->tseg_size)Since it's not bool or pointer, prefer the tseg_size == 0
I don't know if you mean that as indicative or imperative. It is very subjective and for such scenarios I would like to execute my right to mention that it is not part of Contributor Guidelines. I know it might seem rude, but this is one of the things that's very subjective and for such things I like to first reach a consensus before I change what I'm used to writing. I hope you understand that.
+ return; + + virCommandAddArg(cmd, "-global"); + + /* PostParse callback guarantees that the size is divisible by 1 MiB */ + virCommandAddArgFormat(cmd, "mch.extended-tseg-mbytes=%llu", + def->tseg_size >> 20); +} + + static int qemuBuildSmpCommandLine(virCommandPtr cmd, virDomainDefPtr def) @@ -9921,6 +9937,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMachineCommandLine(cmd, cfg, def, qemuCaps) < 0) goto error; + qemuBuildTSEGCommandLine(cmd, def); + if (qemuBuildCpuCommandLine(cmd, driver, def, qemuCaps) < 0) goto error; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 881d0ea46a75..3ea9e3d47344 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3319,6 +3319,87 @@ qemuDomainDefCPUPostParse(virDomainDefPtr def) } +static int +qemuDomainSetDefaultTsegSize(virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + const char *machine = NULL; + char *end_ptr = NULL; + unsigned int major = 0; + unsigned int minor = 0; + + def->tseg_size = 0;Pointless since the only way in here is "if (tseg_size == 0)"
My bad for not commenting the function. But with this line the function is self-sufficient and clearly does what the name suggests (set the default TSEG size) no matter where it is called from and under what circumstances/conditions.
+ + if (!qemuDomainIsQ35(def)) + return 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES))Reading this now makes me realized _MBYTES is probably unnecessary, IDC though since it does follow the QEMU name.
That's exactly why I chose that naming (after changing it many, _many_ times). I imagined there will come a time when QEMU adds an option named mch.extended_tseg(_maybe_some_suffix) and the person adding a capability for that will not adjust the naming on this one. I see this as: a) rather safe than sorry b) very much precise with what it describes
+ return 0; + + machine = STRSKIP(def->os.machine, "pc-q35-"); + + if (!machine) + goto error; + + if (virStrToLong_uip(machine, &end_ptr, 10, &major) < 0) + goto error; + + if (*end_ptr != '.') + goto error; + + machine = end_ptr + 1; + + if (virStrToLong_uip(machine, &end_ptr, 10, &minor) < 0) + goto error; + if (*end_ptr != '\0') + goto error; + + /* QEMU started defaulting to 16MiB after 2.9 */ + if (major > 2 || (major == 2 && minor > 9)) + def->tseg_size = 16 * 1024 * 1024;So, if QEMU defaults to 16MiB, then why do we need so set this at all?
TL;DR because not setting the default value when it is not explicitly requested has already bitten us in the back multiple times. Long story short this way we are safe. No matter what happens (QEMU changing the default, the user changing the config somewhere else and not expecting this to change, us wanting to change the default in the future for some reason, migration to newer libvirt where who-knows-what is checked there, etc.) it is way easier to keep the guest ABI stable. It is visible what the value actually is and we're not hiding it somewhere in the code of the hypervisor. I know we don't do that for many things. I could've gone with the (often alibistic [1]) "the default is left for hypervisor to decide", but since we have the opportunity tomake things right (thanks to Laszlo's explanations), why shouldn't we?
This seems to me we are setting policy which based on history of many patches before is a no go. I'd say NAK to this, unless there is some convincing argument made in the commit message and followup responses to the series (or you can take Jan's R-By and ignore me - your call.
What patches are you referring to? I can add that to the commit message, sure.
+ + return 0; + + error: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse QEMU machine type version '%s'"), + def->os.machine); + return -1; +} + + +static int +qemuDomainDefTsegPostParse(virDomainDefPtr def,While TSEG is what you're adjusting, this is more a 'features' or in particular SMM features post parse callback.
I'm getting the hint of a hidden suggestion from the sentence. Are you
suggesting I should rename the function or create few more layers like
this?
qemuDomainDefSMMFeaturesPostParse() {
qemuDomainDefTsegPostParse();
}
qemuDomainDefFeaturesPostParse() {
qemuDomainDefSMMFeaturesPostParse();
}
qemuDomainDefPostParse() {
...
qemuDomainDefFeaturesPostParse();
...
}
+ virQEMUCapsPtr qemuCaps) +{ + if (def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) + return 0; + + if (!def->tseg_size)Similar... prefer tseg_size == 0
Again, are you directing me to do that or saying what version you like more?
+ return qemuDomainSetDefaultTsegSize(def, qemuCaps); + + if (!qemuDomainIsQ35(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SMM TSEG is only supported with q35 machine type")); + return -1; + } + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting TSEG size is not supported with this QEMU binary")); + return -1; + } + + if (VIR_ROUND_UP(def->tseg_size, 1024 * 1024) != def->tseg_size) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SMM TSEG size must be divisible by 1 MiB")); + return -1; + }Does anywhere else that does a VIR_ROUND_UP elicit an error? Why bother. Curious that this differs from qemuDomainMemoryDeviceAlignSize which claims 1MiB rounding using qemuDomainGetMemorySizeAlignment which returns 1024 unless it's PPC64 which returns 256MiB alignments.
It does. The extended TSEG explicitly allows 1 MiB increments. Not only because of the naming (*_mbytes), but also because bigger granularity would just be pointless probably. Also this is not going to exist on PPC64, it only makes sense with q35.
+ + return 0; +} + + static int qemuDomainDefPostParseBasic(virDomainDefPtr def, virCapsPtr caps, @@ -3389,6 +3470,9 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (qemuDomainDefCPUPostParse(def) < 0) goto cleanup; + if (qemuDomainDefTsegPostParse(def, qemuCaps) < 0) + goto cleanup; + ret = 0; cleanup: virObjectUnref(cfg); diff --git a/tests/qemuxml2argvdata/tseg-explicit-size.args b/tests/qemuxml2argvdata/tseg-explicit-size.args new file mode 100644 index 000000000000..d49c81697e43 --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-explicit-size.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine pc-q35-2.10,accel=tcg,usb=off,smm=on,dump-guest-core=off \ +-global mch.extended-tseg-mbytes=48 \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x1 diff --git a/tests/qemuxml2argvdata/tseg-explicit-size.xml b/tests/qemuxml2argvdata/tseg-explicit-size.xml new file mode 100644 index 000000000000..ae3121048495 --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-explicit-size.xml @@ -0,0 +1,23 @@ +<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='x86_64' machine='pc-q35-2.10'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'> + <tseg>48</tseg>The only difference here from genericxml2xmlindata/tseg.xml is that this one doesn't have "unit='MiB'"
Yes, I wanted to check for the correct parsing of both. Maybe I should put 1 GiB in the other one so that I check that it parses the unit properly? Actually I have KiB somewhere, so probably not.
+ </smm> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/tseg-i440fx.xml b/tests/qemuxml2argvdata/tseg-i440fx.xml new file mode 100644 index 000000000000..5bd832d50829 --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-i440fx.xml @@ -0,0 +1,23 @@ +<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='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'> + <tseg unit='MiB'>48</tseg> + </smm> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/tseg-invalid-size.xml b/tests/qemuxml2argvdata/tseg-invalid-size.xml new file mode 100644 index 000000000000..3ac8069a81ce --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-invalid-size.xml @@ -0,0 +1,23 @@ +<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='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'> + <tseg unit='KiB'>12345</tseg> + </smm> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/tseg-old-machine-type.args b/tests/qemuxml2argvdata/tseg-old-machine-type.args new file mode 100644 index 000000000000..ebbdb15e68f2 --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-old-machine-type.args @@ -0,0 +1,27 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine pc-q35-2.9,accel=tcg,usb=off,smm=on,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x1 diff --git a/tests/qemuxml2argvdata/tseg-old-machine-type.xml b/tests/qemuxml2argvdata/tseg-old-machine-type.xml new file mode 100644 index 000000000000..d1e42586f144 --- /dev/null +++ b/tests/qemuxml2argvdata/tseg-old-machine-type.xml @@ -0,0 +1,21 @@ +<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='x86_64' machine='pc-q35-2.9'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/tseg.args b/tests/qemuxml2argvdata/tseg.args new file mode 100644 index 000000000000..995957ef1d58 --- /dev/null +++ b/tests/qemuxml2argvdata/tseg.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine pc-q35-2.10,accel=tcg,usb=off,smm=on,dump-guest-core=off \ +-global mch.extended-tseg-mbytes=16 \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x1 diff --git a/tests/qemuxml2argvdata/tseg.xml b/tests/qemuxml2argvdata/tseg.xml new file mode 100644 index 000000000000..7a31e348b258 --- /dev/null +++ b/tests/qemuxml2argvdata/tseg.xmland yet another one similr to genericxml2xmlindata/tseg.xml - same name, different directory
Are you suggesting I create this as a symlink? Or that I should add info to the commit message that this patch adds checks for formatting the command-line (the functionality the patch is adding) and that the genericxml2xmltest is checking the parsing even for developers who build with QEMU driver disabled?
@@ -0,0 +1,21 @@ +<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='x86_64' machine='pc-q35-2.10'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <smm state='on'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 78454acb1a41..633dfaaee9a4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2827,6 +2827,54 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-virtio-scsi-reservations"); + DO_TEST("tseg", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES); + DO_TEST("tseg-explicit-size", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES); + DO_TEST("tseg-old-machine-type", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES);These all should use DO_TEST_CAPS_LATEST
OK, good point.
+ DO_TEST_PARSE_ERROR("tseg-i440fx", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES);Failure wrong machine type...+ DO_TEST_PARSE_ERROR("tseg-explicit-size", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI);Failure because TSEG_MBYTES not provided.
I'm sorry, don't take this personally, but sometimes I'm not sure whether you are trying to convey some information or you are just adding comments for yourself. If you are asking me to do something, then please say so. If you want these messages to be there as comments, let me know, I'll do that (although the only time that would be useful is if someone makes a change and the tests start failing (by the tested code not failing) and that person wants to know what was supposed to be the error. In which case they can re-run them without the modifications. Or if you are suggesting that it should be DO_TEST_FAILURE instead of DO_TEST_PARSE_ERROR, then it cannot be, the tests would fail because the code doesn't throw an error during starting the domain, but right away during parsing. And yes, that can be done because I'm not erroring out for something that existed before, but for a new feature so it will never be read from the disk for a domain that existed before this series. Or maybe I ran the tests wrong and it is supposed to be DO_TEST_FAILURE and I made a mistake somewhere else as well. The reason for me not really being sure what you mean by that is that I take all of what's in the previous paragraph as something obvious and known by default for non-newbie libvirt developers (which you most certainly are one since you have years of experience in this codebase), so I'm probably just missing something. It very well might be a language barrier, because I'm nowhere near understanding most of the nuances in English language and various dialects thereof. So with no hidden meaning between the lines, please enlighten me.
+ DO_TEST_PARSE_ERROR("tseg-invalid-size", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES); + /* Test disks with format probing enabled for legacy reasons. * New tests should not go in this section. */ driver.config->allowDiskFormatProbing = true; diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7cedc2b999b3..14824679b81c 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1181,6 +1181,31 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW); + DO_TEST("tseg", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES); + DO_TEST("tseg-explicit-size", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES); + DO_TEST("tseg-old-machine-type", + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_MACHINE_SMM_OPT, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_MCH_EXTENDED_TSEG_MBYTES); +Never quite understood why xml2xml tests need CAPS...
Well, I could rant about that for a while. Basically it is because we want to do some modifications during parsing, so not only tests, but also the rest of the code has capabilities already available when parsing the domein. If you want the non-polite version ask me offline, preferably near a beer tap and far from people who are easily offended. Have a nice day, Martin [1] Why don't you have that as a word already? Someone was trying to suggest that: https://www.collinsdictionary.com/submission/4813/alibistic Maybe because some other languages have it already: https://en.wiktionary.org/wiki/alibistick%C3%BD
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
