[libvirt] [ v3 2/4] 1) Loader: Add a more elaborate definition.
Augment definition to include virStorageSourcePtr that more comprehensively describes the nature of backing element. Also include flags for annotating if input XML definition is old-style or new-style. 2) Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef. This patch is used to interpret domain XML and store the Loader & Nvram's backing definitions as virStorageSource. It also identifies if input XML used old or new-style declaration. (This will later be used by the formatter). 3) Format the loader source appropriately. If the initial XML used the old-style declaration as follows: /path/to/file we format it as was read. However, if it used new-style declaration: The formatter identifies that this is a new-style format and renders it likewise. 4) Plumb the loader source into generation of QEMU command line. Given that nvram & loader elements can now be backed by a non-local source too, adjust all steps leading to generation of QEMU command line. 5) Fix the domain def inference logic to correctly account for network-backed pflash devices. 6) Bhyve: Fix command line generation to correctly pick up local loader path. 7) virt-aa-helper: Adjust references to loader & nvram elements to correctly parse the virStorageSource types. 8) Vbox: Adjust references to 'loader' and 'nvram' elements given that these are now represented by virStorageSourcePtr. 9)Xen: Adjust all references to loader & nvram elements given that they are now backed by virStorageSourcePtr --- src/bhyve/bhyve_command.c | 6 +- src/conf/domain_conf.c | 250 +--- src/conf/domain_conf.h | 11 +- src/qemu/qemu_cgroup.c | 13 ++- src/qemu/qemu_command.c | 21 +++- src/qemu/qemu_domain.c | 31 +++-- src/qemu/qemu_driver.c | 7 +- src/qemu/qemu_parse_command.c | 30 - src/qemu/qemu_process.c | 54 ++--- src/security/security_dac.c | 6 +- src/security/security_selinux.c | 6 +- src/security/virt-aa-helper.c | 14 ++- src/vbox/vbox_common.c | 11 +- src/xenapi/xenapi_driver.c | 4 +- src/xenconfig/xen_sxpr.c| 19 +-- src/xenconfig/xen_xm.c | 9 +- 16 files changed, 409 insertions(+), 83 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index e3f7ded..9e53f40 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -521,10 +521,12 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virCommandAddArgList(cmd, "-s", "0:0,hostbridge", NULL); if (def->os.bootloader == NULL && -def->os.loader) { +def->os.loader && +def->os.loader->src && +def->os.loader->src->type == VIR_STORAGE_TYPE_FILE) { if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { virCommandAddArg(cmd, "-l"); -virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); +virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->src->path); add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3689ac0..df2ed3a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2884,8 +2884,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader) if (!loader) return; -VIR_FREE(loader->path); -VIR_FREE(loader->nvram); +virStorageSourceFree(loader->src); +virStorageSourceFree(loader->nvram); VIR_FREE(loader->templt); VIR_FREE(loader); } @@ -18087,30 +18087,81 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def) static int virDomainLoaderDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, virDomainLoaderDefPtr loader) { int ret = -1; char *readonly_str = NULL; char *secure_str = NULL; char *type_str = NULL; +char *tmp = NULL; +char *lpath = NULL; +xmlNodePtr cur; +xmlXPathContextPtr cur_ctxt = ctxt; + +if (VIR_ALLOC(loader->src) < 0) +goto error; + +loader->src->type = VIR_STORAGE_TYPE_LAST; +loader->oldStyleLoader = false; readonly_str = virXMLPropString(node, "readonly"); secure_str = virXMLPropString(node, "secure"); type_str = virXMLPropString(node, "type"); -loader->path = (char *) xmlNodeGetContent(node); + +if ((tmp = virXMLPropString(node, "backing")) && +(loader->src->type = virStorageTypeFromString(tmp)) <= 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown loader type '%s'"), tmp); +VIR_FREE(tmp); +goto error; +} +VIR_FREE(tmp); + +for (cur = node->children; cur != NULL; cur = cur->next) { +if (cur->type != XML_ELEMENT_NODE) +continue; + +if (virXMLNodeNameEqual(cur, "source")) { +/* new-style declaration foun
[libvirt] [ v3 4/4] Documentation: Add a blurb for the newly added XML snippets for loader and nvram.
Signed-off-by: Prerna Saxena --- docs/formatdomain.html.in | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0d0fd3b..2ba2761 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -112,6 +112,20 @@ </os> ... +Where loader/NVRAM can also be described as: + + +... + <loader readonly='yes' secure='no' type='rom' backing='file'> +<source file='/usr/share/OVMF/OVMF_CODE.fd'/> + </loader> + <nvram backing='network' template='/usr/share/OVMF/OVMF_VARS.fd'> +<source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/2'> + <host name='example.com' port='3260'/> +</source> + </nvram> +... + type The content of the type element specifies the @@ -143,7 +157,15 @@ pflash. Moreover, some firmwares may implement the Secure boot feature. Attribute secure can be used then to control it. -Since 2.1.0 +Since 2.1.0Since 4.5.0 +Loader element can also be specified as a remote disk. The attribute +backing (accepted values are +file and network)can be used to represent +local absolute paths as well as network-backed disks. The details of +backing file may be specified as storage source +Allowed backing type file replaces the old +specification and extends to all hypervisors that supported it, while +type network is only supported by QEMU. nvram Some UEFI firmwares may want to use a non-volatile memory to store some variables. In the host, this is represented as a file and the @@ -154,7 +176,15 @@ from the config file. Note, that for transient domains if the NVRAM file has been created by libvirt it is left behind and it is management application's responsibility to save and remove file (if needed to be -persistent). Since 1.2.8 +persistent). Since 1.2.8 +Since 4.5.0, attribute +backing(accepted values file +and network)can be used to specify a +storage source +of type file or network. The value filedescribes +absolute NVRAM paths and extends the present specification +for all hypervisors that support this, while +network applies only to QEMU. boot The dev attribute takes one of the values "fd", "hd", "cdrom" or "network" and is used to specify the next boot device @@ -2726,7 +2756,7 @@ - source + Disk source Representation of the disk source depends on the disk type attribute value as follows: -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ v3 3/4] Test: Add a test snippet to evaluate command line generation for loader/nvram specified via virStorageSource
Signed-off-by: Prerna Saxena --- tests/qemuxml2argvdata/bios-nvram-network.args | 31 +++ tests/qemuxml2argvdata/bios-nvram-network.xml | 42 ++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 74 insertions(+) create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml diff --git a/tests/qemuxml2argvdata/bios-nvram-network.args b/tests/qemuxml2argvdata/bios-nvram-network.args new file mode 100644 index 000..2f98fe6 --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name test-bios \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,\ +readonly=on \ +-drive file=iscsi://example.org:6000/iqn.1992-01.com.example/0,if=pflash,\ +format=raw,unit=1 \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-test-bios/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot order=c,menu=on \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/bios-nvram-network.xml b/tests/qemuxml2argvdata/bios-nvram-network.xml new file mode 100644 index 000..d0b04ea --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.xml @@ -0,0 +1,42 @@ + + test-bios + 362d1fc1-df7d-193e-5c18-49a71bd1da66 + 1048576 + 1048576 + 1 + +hvm + + + + + + + + + + + + + + + + destroy + restart + restart + +/usr/bin/qemu-system-x86_64 + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 78454ac..e49186c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -853,6 +853,7 @@ mymain(void) QEMU_CAPS_DEVICE_ISA_SERIAL, QEMU_CAPS_SGA); DO_TEST("bios-nvram", NONE); +DO_TEST("bios-nvram-network", NONE); DO_TEST("bios-nvram-secure", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_PCI_BRIDGE, -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ v3 0/4] Introduce network-backed loader & NVRAM.
Libvirt domain XML allows only local filepaths to specify a loader element or its matching NVRAM. Given that VMs may themselves move across hypervisor hosts, it should be possible to allocate loaders/NVRAM disks on network storage for uninterrupted access. This series extends the loader & NVRAM disk elements to be described as virStorageSource* elements, as discussed in : https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html Sample XML with new annotation: References: -- v0/ Proposal: https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html.v1 v1: https://www.redhat.com/archives/libvir-list/2018-April/msg02024.html v2: https://www.redhat.com/archives/libvir-list/2018-May/msg00948.html Changelog: - Changes since v2: - Consolidated patches with related data structures to avoid build breakage. - Passes make check & make syntax-check. Prerna Saxena (4): Schema: Introduce XML schema for network-backed loader and nvram elements. Loader: Add a more elaborate definition. Test: Add a test snippet to evaluate command line generation for loader/nvram specified via virStorageSource Documentation: Add a blurb for the newly added XML snippets for loader and nvram. docs/formatdomain.html.in | 36 +++- docs/schemas/domaincommon.rng | 108 +-- src/bhyve/bhyve_command.c | 6 +- src/conf/domain_conf.c | 250 +++-- src/conf/domain_conf.h | 11 +- src/qemu/qemu_cgroup.c | 13 +- src/qemu/qemu_command.c| 21 ++- src/qemu/qemu_domain.c | 31 ++- src/qemu/qemu_driver.c | 7 +- src/qemu/qemu_parse_command.c | 30 ++- src/qemu/qemu_process.c| 54 -- src/security/security_dac.c| 6 +- src/security/security_selinux.c| 6 +- src/security/virt-aa-helper.c | 14 +- src/vbox/vbox_common.c | 11 +- src/xenapi/xenapi_driver.c | 4 +- src/xenconfig/xen_sxpr.c | 19 +- src/xenconfig/xen_xm.c | 9 +- tests/qemuxml2argvdata/bios-nvram-network.args | 31 +++ tests/qemuxml2argvdata/bios-nvram-network.xml | 42 + tests/qemuxml2argvtest.c | 1 + 21 files changed, 606 insertions(+), 104 deletions(-) create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ v3 1/4] Schema: Introduce XML schema for network-backed loader and nvram elements.
Today, 'loader' and 'nvram' elements are supposed to be backed by a local disk. Given that NVRAM data could be network-backed for high availability, this patch defines XML spec for serving loader & nvram disks via the network. Signed-off-by: Prerna Saxena --- docs/schemas/domaincommon.rng | 108 +++--- 1 file changed, 90 insertions(+), 18 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 71ac3d0..64f4319 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -276,7 +276,42 @@ - + + + + file + network + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -287,7 +322,40 @@ - + + + file + network + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -1499,25 +1567,29 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + block -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/12] Bhyve: Fix command line generation to correctly pick up local loader path.
Signed-off-by: Prerna Saxena --- src/bhyve/bhyve_command.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 9413ae5..2b67014 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -518,10 +518,12 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virCommandAddArgList(cmd, "-s", "0:0,hostbridge", NULL); if (def->os.bootloader == NULL && -def->os.loader) { +def->os.loader && +def->os.loader->src && +def->os.loader->src->type == VIR_STORAGE_TYPE_FILE) { if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { virCommandAddArg(cmd, "-l"); -virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); +virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->src->path); add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/12] Vbox: Adjust references to 'loader' and 'nvram' elements given that these are now represented by virStorageSourcePtr.
Signed-off-by: Prerna Saxena --- src/vbox/vbox_common.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 72a24a3..60451a3 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -998,11 +998,16 @@ vboxSetBootDeviceOrder(virDomainDefPtr def, vboxDriverPtr data, VIR_DEBUG("def->os.initrd %s", def->os.initrd); VIR_DEBUG("def->os.cmdline %s", def->os.cmdline); VIR_DEBUG("def->os.root %s", def->os.root); -if (def->os.loader) { -VIR_DEBUG("def->os.loader->path %s", def->os.loader->path); +if (def->os.loader && +def->os.loader->src && +def->os.loader->src->type == VIR_STORAGE_TYPE_FILE) { + +VIR_DEBUG("def->os.loader->src->path %s", def->os.loader->src->path); VIR_DEBUG("def->os.loader->readonly %d", def->os.loader->readonly); VIR_DEBUG("def->os.loader->type %d", def->os.loader->type); -VIR_DEBUG("def->os.loader->nvram%s", def->os.loader->nvram); +if (def->os.loader->nvram && +def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE) +VIR_DEBUG("def->os.loader->nvram->path%s", def->os.loader->nvram->path); } VIR_DEBUG("def->os.bootloader %s", def->os.bootloader); VIR_DEBUG("def->os.bootloaderArgs %s", def->os.bootloaderArgs); -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/12] Test: Add a test snippet to evaluate command line generation for loader/nvram specified via virStorageSource
Signed-off-by: Prerna Saxena --- tests/qemuxml2argvdata/bios-nvram-network.args | 31 +++ tests/qemuxml2argvdata/bios-nvram-network.xml | 42 ++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 74 insertions(+) create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml diff --git a/tests/qemuxml2argvdata/bios-nvram-network.args b/tests/qemuxml2argvdata/bios-nvram-network.args new file mode 100644 index 000..3fc68ef --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name test-bios \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,\ +readonly=on \ +-drive file=iscsi://example.org:6000/iqn.1992-01.com.example/0,\ +if=pflash,format=raw,unit=1 \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-test-bios/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot order=c,menu=on \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/bios-nvram-network.xml b/tests/qemuxml2argvdata/bios-nvram-network.xml new file mode 100644 index 000..bad114c --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.xml @@ -0,0 +1,42 @@ + + test-bios + 362d1fc1-df7d-193e-5c18-49a71bd1da66 + 1048576 + 1048576 + 1 + +hvm + + + + + + + + + + + + + + + + destroy + restart + restart + +/usr/bin/qemu-system-x86_64 + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ddf567b..7338dba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -853,6 +853,7 @@ mymain(void) QEMU_CAPS_DEVICE_ISA_SERIAL, QEMU_CAPS_SGA); DO_TEST("bios-nvram", NONE); +DO_TEST("bios-nvram-network", NONE); DO_TEST("bios-nvram-secure", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_PCI_BRIDGE, -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/12] Plumb the loader source into generation of QEMU command line.
Given that nvram & loader elements can now be backed by a non-local source too, adjust all steps leading to generation of QEMU command line. Signed-off-by: Prerna Saxena --- src/qemu/qemu_cgroup.c | 13 + src/qemu/qemu_command.c | 21 - src/qemu/qemu_domain.c | 31 +- src/qemu/qemu_driver.c | 7 --- src/qemu/qemu_process.c | 42 - src/security/security_dac.c | 6 -- src/security/security_selinux.c | 6 -- 7 files changed, 88 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index d88eb78..2068eb0 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -580,16 +580,21 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm) static int qemuSetupFirmwareCgroup(virDomainObjPtr vm) { +virStorageSourcePtr src = NULL; + if (!vm->def->os.loader) return 0; -if (vm->def->os.loader->path && -qemuSetupImagePathCgroup(vm, vm->def->os.loader->path, - vm->def->os.loader->readonly == VIR_TRISTATE_BOOL_YES) < 0) +src = vm->def->os.loader->src; + +if (src->type == VIR_STORAGE_TYPE_FILE && +qemuSetupImagePathCgroup(vm, src->path, + src->readonly == VIR_TRISTATE_BOOL_YES) < 0) return -1; if (vm->def->os.loader->nvram && -qemuSetupImagePathCgroup(vm, vm->def->os.loader->nvram, false) < 0) +vm->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE && +qemuSetupImagePathCgroup(vm, vm->def->os.loader->nvram->path, false) < 0) return -1; return 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 08f67a4..e9d6e4b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9260,6 +9260,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainLoaderDefPtr loader = def->os.loader; virBuffer buf = VIR_BUFFER_INITIALIZER; int unit = 0; +char *source = NULL; if (!loader) return; @@ -9267,7 +9268,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, switch ((virDomainLoader) loader->type) { case VIR_DOMAIN_LOADER_TYPE_ROM: virCommandAddArg(cmd, "-bios"); -virCommandAddArg(cmd, loader->path); +virCommandAddArg(cmd, loader->src->path); break; case VIR_DOMAIN_LOADER_TYPE_PFLASH: @@ -9279,9 +9280,14 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, NULL); } +if (qemuGetDriveSourceString(loader->src, NULL, &source) < 0) +break; + virBufferAddLit(&buf, "file="); -virQEMUBuildBufferEscapeComma(&buf, loader->path); -virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", unit); +virQEMUBuildBufferEscapeComma(&buf, source); +free(source); +virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", + unit); unit++; if (loader->readonly) { @@ -9294,9 +9300,14 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, if (loader->nvram) { virBufferFreeAndReset(&buf); +if (qemuGetDriveSourceString(loader->nvram, NULL, &source) < 0) +break; + virBufferAddLit(&buf, "file="); -virQEMUBuildBufferEscapeComma(&buf, loader->nvram); -virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", unit); +virQEMUBuildBufferEscapeComma(&buf, source); +virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", + unit); +unit++; virCommandAddArg(cmd, "-drive"); virCommandAddArgBuffer(cmd, &buf); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9bb6d8a..2d4e299 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3318,6 +3318,9 @@ qemuDomainDefPostParse(virDomainDefPtr def, * function shall not fail in that case. It will be re-run on VM startup * with the capabilities populated. */ virQEMUCapsPtr qemuCaps = parseOpaque; +virDomainLoaderDefPtr ldr = NULL; +char *nvramPath = NULL; + int ret = -1; if (def->os.bootloader || def->os.bootloaderArgs) { @@ -3332,13 +3335,20 @@ qemuDomainDefPostParse(virDomainDefPtr def, goto cleanup; } -if (def->os.loader && -def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH && -def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON && -!def->
[libvirt] [PATCH 06/12] Fix the domain def inference logic to correctly account for network-backed pflash devices.
Signed-off-by: Prerna Saxena --- src/qemu/qemu_parse_command.c | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 351425f..9b1a86e 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -650,6 +650,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, int idx = -1; int busid = -1; int unitid = -1; +bool is_firmware = false; if (qemuParseKeywords(val, &keywords, @@ -772,6 +773,9 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO; } else if (STREQ(values[i], "xen")) { def->bus = VIR_DOMAIN_DISK_BUS_XEN; +} else if (STREQ(values[i], "pflash")) { +def->bus = VIR_DOMAIN_DISK_BUS_LAST; +is_firmware = true; } else if (STREQ(values[i], "sd")) { def->bus = VIR_DOMAIN_DISK_BUS_SD; } @@ -943,8 +947,25 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, ignore_value(VIR_STRDUP(def->dst, "hda")); } -if (!def->dst) -goto error; +if (!def->dst) { +if (is_firmware && def->bus == VIR_DOMAIN_DISK_BUS_LAST) { +if (!dom->os.loader && (VIR_ALLOC(dom->os.loader) < 0)) +goto error; +if (def->src->readonly) { +/* Loader spec */ +dom->os.loader->src = def->src; +dom->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; +} else { +/* NVRAM Spec */ +if (!dom->os.loader->nvram && (VIR_ALLOC(dom->os.loader->nvram) < 0)) +goto error; +dom->os.loader->nvram = def->src; +} +} else { +goto error; +} +} + if (STREQ(def->dst, "xvda")) def->dst[3] = 'a' + idx; else @@ -2215,8 +2236,11 @@ qemuParseCommandLine(virCapsPtr caps, } else if (STREQ(arg, "-bios")) { WANT_VALUE(); if (VIR_ALLOC(def->os.loader) < 0 || -VIR_STRDUP(def->os.loader->path, val) < 0) +VIR_ALLOC(def->os.loader->src) < 0 || +VIR_STRDUP(def->os.loader->src->path, val) < 0) goto error; +def->os.loader->src->type = VIR_STORAGE_TYPE_FILE; +def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_ROM; } else if (STREQ(arg, "-initrd")) { WANT_VALUE(); if (VIR_STRDUP(def->os.initrd, val) < 0) -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/12] Format the loader source appropriately.
If the initial XML used the old-style declaration as follows: /path/to/file we format it as was read. However, if it used new-style declaration: The formatter identifies that this is a new-style format and renders it likewise. Signed-off-by: Prerna Saxena --- src/conf/domain_conf.c | 86 -- 1 file changed, 76 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index be43695..d59a579 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18094,7 +18094,7 @@ virDomainLoaderNvramDefParseXML(xmlNodePtr node, } loader->nvram->type = VIR_STORAGE_TYPE_LAST; -loader->nvram->oldStyleNvram = false; +loader->oldStyleNvram = false; if ((tmp = virXMLPropString(node, "backing")) && (loader->nvram->type = virStorageTypeFromString(tmp)) <= 0) { @@ -26212,11 +26212,19 @@ virDomainHugepagesFormat(virBufferPtr buf, static void virDomainLoaderDefFormat(virBufferPtr buf, - virDomainLoaderDefPtr loader) + virDomainLoaderDefPtr loader, + unsigned int flags) { const char *readonly = virTristateBoolTypeToString(loader->readonly); const char *secure = virTristateBoolTypeToString(loader->secure); const char *type = virDomainLoaderTypeToString(loader->type); +const char *backing = NULL; + +virBuffer attrBuf = VIR_BUFFER_INITIALIZER; +virBuffer childBuf = VIR_BUFFER_INITIALIZER; + +virBufferSetChildIndent(&childBuf, buf); + virBufferAddLit(buf, "secure) virBufferAsprintf(buf, " secure='%s'", secure); -virBufferAsprintf(buf, " type='%s'>", type); +virBufferAsprintf(buf, " type='%s'", type); +if (loader->src && +loader->src->type < VIR_STORAGE_TYPE_LAST) { +if (!loader->oldStyleLoader) { +/* Format this in the new style, using the + * sub-element */ +if (virDomainStorageSourceFormat(&attrBuf, &childBuf, loader->src, + flags, 0) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format loader source")); +goto cleanup; +} + +backing = virStorageTypeToString(loader->src->type); +virBufferAsprintf(buf, " backing='%s'>", backing); + +if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format loader source")); +goto cleanup; +} + +} else +/* Format this in the old-style, using absolute paths directly. */ +virBufferAsprintf(buf, ">%s", loader->src->path); +} else +virBufferAddLit(buf, ">\n"); + +virBufferAddLit(buf, "\n"); -virBufferEscapeString(buf, "%s\n", loader->path); if (loader->nvram || loader->templt) { +ignore_value(virBufferContentAndReset(&attrBuf)); +ignore_value(virBufferContentAndReset(&childBuf)); +virBufferSetChildIndent(&childBuf, buf); + virBufferAddLit(buf, "templt); -if (loader->nvram) -virBufferEscapeString(buf, ">%s\n", loader->nvram); -else -virBufferAddLit(buf, "/>\n"); + +if (loader->templt) +virBufferEscapeString(buf, " template='%s'", loader->templt); + +if (loader->nvram) { +backing = virStorageTypeToString(loader->nvram->type); +if (!loader->oldStyleNvram) { +if (virDomainStorageSourceFormat(&attrBuf, &childBuf, + loader->nvram, flags, 0) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format NVRAM source")); +virBufferAddLit(buf, ">\n\n"); +goto cleanup; +} + +virBufferEscapeString(buf, " backing='%s'>", backing); +if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format NVRAM source")); +virBufferAddLit(buf, "\n"); +goto cleanup; +} +} else { +/* old-style NVRAM declaration found
[libvirt] [PATCH 08/12] virt-aa-helper: Adjust references to loader & nvram elements to correctly parse the virStorageSource types.
Signed-off-by: Prerna Saxena --- src/security/virt-aa-helper.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index d0f9876..8217d67 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1063,12 +1063,18 @@ get_files(vahControl * ctl) if (vah_add_file(&buf, ctl->def->os.slic_table, "r") != 0) goto cleanup; -if (ctl->def->os.loader && ctl->def->os.loader->path) -if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0) +if (ctl->def->os.loader && +ctl->def->os.loader->src && +ctl->def->os.loader->src->type == VIR_STORAGE_TYPE_FILE && +ctl->def->os.loader->src->path) +if (vah_add_file(&buf, ctl->def->os.loader->src->path, "rk") != 0) goto cleanup; -if (ctl->def->os.loader && ctl->def->os.loader->nvram) -if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rwk") != 0) +if (ctl->def->os.loader && +ctl->def->os.loader->nvram && +ctl->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE && +ctl->def->os.loader->nvram->path) +if (vah_add_file(&buf, ctl->def->os.loader->nvram->path, "rwk") != 0) goto cleanup; for (i = 0; i < ctl->def->ngraphics; i++) { -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/12] Schema: Introduce XML schema for network-backed loader and nvram elements.
Today, 'loader' and 'nvram' elements are supposed to be backed by a local disk. Given that NVRAM data could be network-backed for high availability, this patch defines XML spec for serving loader & nvram disks via the network. Signed-off-by: Prerna Saxena --- docs/schemas/domaincommon.rng | 108 +++--- 1 file changed, 90 insertions(+), 18 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0a6b29b..a6207db 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -276,7 +276,42 @@ - + + + + file + network + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -287,7 +322,40 @@ - + + + file + network + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -1494,25 +1562,29 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + block -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/12] Xen: Adjust all references to loader & nvram elements given that they are now backed by virStorageSourcePtr
Signed-off-by: Prerna Saxena --- src/xenapi/xenapi_driver.c | 4 +++- src/xenconfig/xen_sxpr.c | 19 +++ src/xenconfig/xen_xm.c | 9 ++--- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 42b305d..4070660 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1444,10 +1444,12 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) char *value = NULL; defPtr->os.type = VIR_DOMAIN_OSTYPE_XEN; if (VIR_ALLOC(defPtr->os.loader) < 0 || -VIR_STRDUP(defPtr->os.loader->path, "pygrub") < 0) { +VIR_ALLOC(defPtr->os.loader->src) < 0 || +VIR_STRDUP(defPtr->os.loader->src->path, "pygrub") < 0) { VIR_FREE(boot_policy); goto error; } +defPtr->os.loader->src->type = VIR_STORAGE_TYPE_FILE; xen_vm_get_pv_kernel(session, &value, vm); if (STRNEQ(value, "")) { if (VIR_STRDUP(defPtr->os.kernel, value) < 0) { diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index e868c05..fd3165c 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -87,15 +87,17 @@ xenParseSxprOS(const struct sexpr *node, int hvm) { if (hvm) { -if (VIR_ALLOC(def->os.loader) < 0) +if ((VIR_ALLOC(def->os.loader) < 0) || +(VIR_ALLOC(def->os.loader->src) < 0)) goto error; -if (sexpr_node_copy(node, "domain/image/hvm/loader", &def->os.loader->path) < 0) +def->os.loader->src->type = VIR_STORAGE_TYPE_FILE; +if (sexpr_node_copy(node, "domain/image/hvm/loader", &def->os.loader->src->path) < 0) goto error; -if (def->os.loader->path == NULL) { -if (sexpr_node_copy(node, "domain/image/hvm/kernel", &def->os.loader->path) < 0) +if (def->os.loader->src->path == NULL) { +if (sexpr_node_copy(node, "domain/image/hvm/kernel", &def->os.loader->src->path) < 0) goto error; -if (def->os.loader->path == NULL) { +if (def->os.loader->src->path == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain information incomplete, missing HVM loader")); return -1; @@ -124,7 +126,8 @@ xenParseSxprOS(const struct sexpr *node, /* If HVM kenrel == loader, then old xend, so kill off kernel */ if (hvm && def->os.kernel && -STREQ(def->os.kernel, def->os.loader->path)) { +def->os.loader->src && +STREQ(def->os.kernel, def->os.loader->src->path)) { VIR_FREE(def->os.kernel); } /* Drop kernel argument that has no value */ @@ -2259,9 +2262,9 @@ xenFormatSxpr(virConnectPtr conn, virDomainDefPtr def) if (hvm) { char bootorder[VIR_DOMAIN_BOOT_LAST+1]; if (def->os.kernel) -virBufferEscapeSexpr(&buf, "(loader '%s')", def->os.loader->path); +virBufferEscapeSexpr(&buf, "(loader '%s')", def->os.loader->src->path); else -virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.loader->path); +virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.loader->src->path); virBufferAsprintf(&buf, "(vcpus %u)", virDomainDefGetVcpusMax(def)); if (virDomainDefHasVcpusOffline(def)) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index 8ef68bb..c6a1f03 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -47,8 +47,10 @@ xenParseXMOS(virConfPtr conf, virDomainDefPtr def) const char *boot; if (VIR_ALLOC(def->os.loader) < 0 || -xenConfigCopyString(conf, "kernel", &def->os.loader->path) < 0) +VIR_ALLOC(def->os.loader->src) < 0 || +xenConfigCopyString(conf, "kernel", &def->os.loader->src->path) < 0) return -1; +def->os.loader->src->type = VIR_STORAGE_TYPE_FILE; if (xenConfigGetString(conf, "boot", &boot, "c") < 0) return -1; @@ -481,9 +483,10 @@ xenFormatXMOS(virConfPtr conf, virDomainDefPtr def) if (xenConfigSetString(conf, "builder", "hvm") < 0) return -1; -if (def->os.loader && def->os.loader->path && -xenConfigSetString(conf, "kernel", def->os.loader->path) < 0) +if (def->os.loader && def->os.loader->src && +xenConfigSetString(conf, "kernel", def->os.loader->src->path) < 0) return -1; +def->os.loader->src->type = VIR_STORAGE_TYPE_FILE; for (i = 0; i < def->os.nBootDevs; i++) { switch (def->os.bootDevs[i]) { -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/12] Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef.
This patch is used to interpret domain XML and store the Loader & Nvram's backing definitions as virStorageSource. It also identifies if input XML used old or new-style declaration. (This will later be used by the formatter). Signed-off-by: Prerna Saxena --- src/conf/domain_conf.c | 131 ++--- 1 file changed, 124 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f678e26..be43695 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2884,8 +2884,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader) if (!loader) return; -VIR_FREE(loader->path); -VIR_FREE(loader->nvram); +virStorageSourceFree(loader->src); +virStorageSourceFree(loader->nvram); VIR_FREE(loader->templt); VIR_FREE(loader); } @@ -17986,17 +17986,62 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def) static int virDomainLoaderDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, virDomainLoaderDefPtr loader) { int ret = -1; char *readonly_str = NULL; char *secure_str = NULL; char *type_str = NULL; +char *tmp = NULL; +xmlNodePtr cur; +xmlXPathContextPtr cur_ctxt = ctxt; + +if (VIR_ALLOC(loader->src)) { +goto cleanup; +} +loader->src->type = VIR_STORAGE_TYPE_LAST; +loader->oldStyleLoader = false; readonly_str = virXMLPropString(node, "readonly"); secure_str = virXMLPropString(node, "secure"); type_str = virXMLPropString(node, "type"); -loader->path = (char *) xmlNodeGetContent(node); + +if ((tmp = virXMLPropString(node, "backing")) && +(loader->src->type = virStorageTypeFromString(tmp)) <= 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown loader type '%s'"), tmp); +goto cleanup; +} +VIR_FREE(tmp); + +for (cur = node->children; cur != NULL; cur = cur->next) { +if (cur->type != XML_ELEMENT_NODE) { +continue; +} + +if (virXMLNodeNameEqual(cur, "source")) { +/* new-style declaration found */ +if (virDomainStorageSourceParse(cur, cur_ctxt, loader->src, 0) < 0) { +virReportError(VIR_ERR_XML_DETAIL, + _("Error parsing Loader source element")); +goto cleanup; +} +break; +} +} + +/* Old-style absolute path found ? */ +if (loader->src->type == VIR_STORAGE_TYPE_LAST) { +if (!(loader->src->path = (char *) xmlNodeGetContent(node))) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing loader source")); +goto cleanup; +} else { +loader->src->type = VIR_STORAGE_TYPE_FILE; +loader->oldStyleLoader = true; +} +} if (readonly_str && (loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= 0) { @@ -18023,13 +18068,81 @@ virDomainLoaderDefParseXML(xmlNodePtr node, } ret = 0; - cleanup: +goto exit; +cleanup: +if (loader->src) + VIR_FREE(loader->src); +exit: VIR_FREE(readonly_str); VIR_FREE(secure_str); VIR_FREE(type_str); + return ret; } +static int +virDomainLoaderNvramDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virDomainLoaderDefPtr loader) +{ +int ret = -1; +char *tmp = NULL; +xmlNodePtr cur; + +if (VIR_ALLOC(loader->nvram)) { +goto cleanup; +} + +loader->nvram->type = VIR_STORAGE_TYPE_LAST; +loader->nvram->oldStyleNvram = false; + +if ((tmp = virXMLPropString(node, "backing")) && +(loader->nvram->type = virStorageTypeFromString(tmp)) <= 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown nvram type '%s'"), tmp); +goto cleanup; +} +VIR_FREE(tmp); + +for (cur = node->children; cur != NULL; cur = cur->next) { +if (cur->type != XML_ELEMENT_NODE) { +continue; +} + +if (virXMLNodeNameEqual(cur, "template")) { +loader->templt = virXPathString("string(./os/nvram[1]/@template)", ctxt); +continue; +} + +if (virXMLNodeNameEqual(cur, "source")) { +if (virDomainStorageSourceParse(cur, ctxt, loader->nvram, 0) < 0) { +virReportError(VIR_ERR_XML_DETAIL, + _("Error parsing nvram source element")); +goto cleanup;
[libvirt] [PATCH 12/12] Documentation: Add a blurb for the newly added XML snippets for loader and nvram.
Signed-off-by: Prerna Saxena --- docs/formatdomain.html.in | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index caeb14e..b8cb7ac 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -112,6 +112,20 @@ </os> ... +Where loader/NVRAM can also be described as: + + +... + <loader readonly='yes' secure='no' type='rom' backing='file'> +<source file='/usr/share/OVMF/OVMF_CODE.fd'/> + </loader> + <nvram backing='network' template='/usr/share/OVMF/OVMF_VARS.fd'> +<source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/2'> + <host name='example.com' port='3260'/> +</source> + </nvram> +... + type The content of the type element specifies the @@ -143,7 +157,15 @@ pflash. Moreover, some firmwares may implement the Secure boot feature. Attribute secure can be used then to control it. -Since 2.1.0 +Since 2.1.0Since 4.5.0 +Loader element can also be specified as a remote disk. The attribute +backing (accepted values are +file and network)can be used to represent +local absolute paths as well as network-backed disks. The details of +backing file may be specified as storage source +Allowed backing type file replaces the old +specification and extends to all hypervisors that supported it, while +type network is only supported by QEMU. nvram Some UEFI firmwares may want to use a non-volatile memory to store some variables. In the host, this is represented as a file and the @@ -154,7 +176,15 @@ from the config file. Note, that for transient domains if the NVRAM file has been created by libvirt it is left behind and it is management application's responsibility to save and remove file (if needed to be -persistent). Since 1.2.8 +persistent). Since 1.2.8 +Since 4.5.0, attribute +backing(accepted values file +and network)can be used to specify a +storage source +of type file or network. The value filedescribes +absolute NVRAM paths and extends the present specification +for all hypervisors that support this, while +network applies only to QEMU. boot The dev attribute takes one of the values "fd", "hd", "cdrom" or "network" and is used to specify the next boot device @@ -2707,7 +2737,7 @@ - source + Disk source Representation of the disk source depends on the disk type attribute value as follows: -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/12][v2] Introduce network-backed loader & nvram.
Libvirt domain XML today only allows local filepaths that can be used to specify a loader element or its matching NVRAM disk. Given that Vms may themselves move across hypervisor hosts, it should be possible to allocate loaders/NVRAM disks on network storage for uninterrupted access. This series extends the firmware loader & NVRAM disks to be hosted over network-backed disks, for high availability. It achieves this by embedding virStorageSource elements for loader & nvram into _virDomainLoaderDef, as discussed in https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html. Currently, the source type is annotated by introducing a new attribute "backing" for both 'loader' and 'nvram' elements. Hence, a sample XML with new annotation looks like this: References: -- v0 / Proposal: https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html. v1 : https://www.redhat.com/archives/libvir-list/2018-April/msg02024.html Changelog: - Changes since v1: 1. Split up the patch into smaller units. 2. Added doc snippet & tests. 3. Changed the formatting code in patch 4 to format a domain's XML in the style that was read. I found that encryption seems to be a property of the storage volume. I didnt include that in this series since it does not use backing type as volume. Will include that later once the basic network support patches get done. Looking forward to feedback, Prerna Prerna Saxena (12): Schema: Introduce XML schema for network-backed loader and nvram elements. Loader: Add a more elaborate definition. Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef. Format the loader source appropriately. Plumb the loader source into generation of QEMU command line. Fix the domain def inference logic to correctly account for network-backed pflash devices. Bhyve: Fix command line generation to correctly pick up local loader path. virt-aa-helper: Adjust references to loader & nvram elements to correctly parse the virStorageSource types. Vbox: Adjust references to 'loader' and 'nvram' elements given that these are now represented by virStorageSourcePtr. Xen: Adjust all references to loader & nvram elements given that they are now backed by virStorageSourcePtr Test: Add a test snippet to evaluate command line generation for loader/nvram specified via virStorageSource Documentation: Add a blurb for the newly added XML snippets for loader and nvram. docs/formatdomain.html.in | 36 - docs/schemas/domaincommon.rng | 108 ++--- src/bhyve/bhyve_command.c | 6 +- src/conf/domain_conf.c | 215 +++-- src/conf/domain_conf.h | 11 +- src/qemu/qemu_cgroup.c | 13 +- src/qemu/qemu_command.c| 21 ++- src/qemu/qemu_domain.c | 31 ++-- src/qemu/qemu_driver.c | 7 +- src/qemu/qemu_parse_command.c | 30 +++- src/qemu/qemu_process.c| 42 +++-- src/security/security_dac.c| 6 +- src/security/security_selinux.c| 6 +- src/security/virt-aa-helper.c | 14 +- src/vbox/vbox_common.c | 11 +- src/xenapi/xenapi_driver.c | 4 +- src/xenconfig/xen_sxpr.c | 19 ++- src/xenconfig/xen_xm.c | 9 +- tests/qemuxml2argvdata/bios-nvram-network.args | 31 tests/qemuxml2argvdata/bios-nvram-network.xml | 42 + tests/qemuxml2argvtest.c | 1 + 21 files changed, 562 insertions(+), 101 deletions(-) create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/12] Loader: Add a more elaborate definition.
Augment definition to include virStorageSourcePtr that more comprehensively describes the nature of backing element. Also include flags for annotating if input XML definition is old-style or new-style. Signed-off-by: Prerna Saxena --- src/conf/domain_conf.h | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 15d228b..bbd021b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1855,15 +1855,22 @@ typedef enum { VIR_ENUM_DECL(virDomainLoader) +struct _virStorageSource; +typedef struct _virStorageSource* virStorageSourcePtr; + typedef struct _virDomainLoaderDef virDomainLoaderDef; typedef virDomainLoaderDef *virDomainLoaderDefPtr; struct _virDomainLoaderDef { -char *path; +virStorageSourcePtr src; int readonly; /* enum virTristateBool */ virDomainLoader type; int secure; /* enum virTristateBool */ -char *nvram;/* path to non-volatile RAM */ +virStorageSourcePtr nvram; /* path to non-voliatile RAM */ char *templt; /* user override of path to master nvram */ +bool oldStyleLoader; /* is this an old-style XML formatting, + * ie, absolute path is directly specified? */ +bool oldStyleNvram; /* is this an old-style XML formatting, + * ie, absolute path is directly specified? */ }; void virDomainLoaderDefFree(virDomainLoaderDefPtr loader); -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Qemu driver: Support network-backed pflash disks.
So far libvirt domain XML only allows local filepaths that can be used to specify a loader element or its matching NVRAM disk. Given that Vms may themselves move across hypervisor hosts, it should be possible to allocate loaders/NVRAM disks on network storage for uninterrupted access. Signed-off-by: Prerna Saxena --- docs/schemas/domaincommon.rng | 108 +++ src/conf/domain_conf.c | 188 src/conf/domain_conf.h | 7 +- src/qemu/qemu_cgroup.c | 13 ++- src/qemu/qemu_command.c | 21 +++-- src/qemu/qemu_domain.c | 16 ++-- src/qemu/qemu_driver.c | 7 +- src/qemu/qemu_parse_command.c | 30 ++- src/qemu/qemu_process.c | 33 --- src/security/security_dac.c | 6 +- src/security/security_selinux.c | 6 +- 11 files changed, 361 insertions(+), 74 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4cab55f..32db395 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -276,7 +276,42 @@ - + + + + file + network + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -287,7 +322,40 @@ - + + + file + network + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -1494,25 +1562,29 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + block diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 35666c1..c80f9d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2883,8 +2883,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader) if (!loader) return; -VIR_FREE(loader->path); -VIR_FREE(loader->nvram); +virStorageSourceFree(loader->loader_src); +virStorageSourceFree(loader->nvram); VIR_FREE(loader->templt); VIR_FREE(loader); } @@ -17961,17 +17961,59 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def) static int virDomainLoaderDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, virDomainLoaderDefPtr loader) { int ret = -1; char *readonly_str = NULL; char *secure_str = NULL; char *type_str = NULL; +char *tmp = NULL; +xmlNodePtr cur; +xmlXPathContextPtr cur_ctxt = ctxt; + +if (VIR_ALLOC(loader->loader_src)) { +goto cleanup; +} +loader->loader_src->type = VIR_STORAGE_TYPE_LAST; readonly_str = virXMLPropString(node, "readonly"); secure_str = virXMLPropString(node, "secure"); type_str = virXMLPropString(node, "type"); -loader->path = (char *) xmlNodeGetContent(node); + +if ((tmp = virXMLPropString(node, "backing")) && +(loader->loader_src->type = virStorageTypeFromString(tmp)) <= 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown loader type '%s'"), tmp); +goto cleanup; +} +VIR_FREE(tmp); + +for (cur = node->children; cur != NULL; cur = cur->next) { +if (cur->type != XML_ELEMENT_NODE) { +continue; +} + +if (virXMLNodeNameEqual(cur, "source")) { +if (virDomainStorageSourceParse(cur, cur_ctxt, loader->load
[libvirt] [PATCH] Qemu driver: Support network-backed pflash disks.
This implements support for firmware loader & NVRAM disks over network-backed disks. As discussed in https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html, the patch embeds the spec for disks in and elements as well. Currently, the source type is annotated by introducing a new attribute "backing" for both 'loader' and 'nvram' elements. Hence, a sample XML with new annotation looks like this: The patche automatically re-formats any older-stype declaration into this new style. Templates can be used to create a new NVRAM only if the nvram backing = 'file'. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Migration: Preserve the failed job in case migration job is terminated beyond the perform phase.
In case of non-p2p migration, in case libvirt client gets disconnected from source libvirt after PERFORM phase is over, the daemon just resets the current migration job. However, the VM could be left paused on both source and destination in such case. In case the client reconnects and queries migration status, the job has been blanked out from source libvirt, and this reconnected client has no clear way of figuring out if an unclean migration had previously been aborted. This patch calls out a "potentially" incomplete migration as a failed job, so that a client may be able to watch previously failed jobs for this VM and take corrective actions as needed. Signed-off-by: Prerna Saxena --- src/qemu/qemu_domain.c| 16 src/qemu/qemu_domain.h|2 ++ src/qemu/qemu_migration.c |4 ++-- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e8e0313..7c60d17 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4564,6 +4564,22 @@ qemuDomainObjDiscardAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj) qemuDomainObjSaveJob(driver, obj); } + +void +qemuDomainObjFailAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj) +{ +qemuDomainObjPrivatePtr priv = obj->privateData; +VIR_FREE(priv->job.completed); +if (VIR_ALLOC(priv->job.completed) == 0) { +priv->job.current->type = VIR_DOMAIN_JOB_FAILED; +priv->job.completed = priv->job.current; +} else { +VIR_WARN("Unable to allocate job.completed for VM %s", obj->def->name); +} +qemuDomainObjResetAsyncJob(priv); +qemuDomainObjEndJob(driver, obj); +} + void qemuDomainObjReleaseAsyncJob(virDomainObjPtr obj) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c33af36..6465603 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -497,6 +497,8 @@ void qemuDomainObjRestoreJob(virDomainObjPtr obj, struct qemuDomainJobObj *job); void qemuDomainObjDiscardAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj); +void qemuDomainObjFailAsyncJob(virQEMUDriverPtr driver, + virDomainObjPtr obj); void qemuDomainObjReleaseAsyncJob(virDomainObjPtr obj); qemuMonitorPtr qemuDomainGetMonitor(virDomainObjPtr vm) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 69eb231..fd8950e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1911,8 +1911,8 @@ qemuMigrationCleanup(virDomainObjPtr vm, VIR_WARN("Migration of domain %s finished but we don't know if the" " domain was successfully started on destination or not", vm->def->name); -/* clear the job and let higher levels decide what to do */ -qemuDomainObjDiscardAsyncJob(driver, vm); +/* clearly "fail" the job and let higher levels decide what to do */ +qemuDomainObjFailAsyncJob(driver, vm); break; case QEMU_MIGRATION_PHASE_PERFORM3: -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [[RFC] 2/8] QEMU Event handling: Introduce async event helpers in qemu_event.[ch]
These contain basic functions for setting up event lists (global as well as per-VM). Also include methods for enqueuing and dequeuing events. Per-event metadata is also encoded herewith. Signed-off-by: Prerna Saxena --- src/Makefile.am | 1 + src/qemu/qemu_event.c | 75 + src/qemu/qemu_event.h | 224 ++ 3 files changed, 300 insertions(+) create mode 100644 src/qemu/qemu_event.c create mode 100644 src/qemu/qemu_event.h diff --git a/src/Makefile.am b/src/Makefile.am index b74856b..73a98ca 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -903,6 +903,7 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_domain.c qemu/qemu_domain.h \ qemu/qemu_domain_address.c qemu/qemu_domain_address.h \ qemu/qemu_cgroup.c qemu/qemu_cgroup.h \ + qemu/qemu_event.c qemu/qemu_event.h \ qemu/qemu_hostdev.c qemu/qemu_hostdev.h \ qemu/qemu_hotplug.c qemu/qemu_hotplug.h \ qemu/qemu_hotplugpriv.h \ diff --git a/src/qemu/qemu_event.c b/src/qemu/qemu_event.c new file mode 100644 index 000..e27ea0d --- /dev/null +++ b/src/qemu/qemu_event.c @@ -0,0 +1,75 @@ +/* + * qemu_event.c: + *optimize qemu async event handling. + * + * Copyright (C) 2017-2026 Nutanix, Inc. + * Copyright (C) 2017 Prerna Saxena + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Prerna Saxena + */ + +#include "config.h" +#include "internal.h" +# include "qemu_monitor.h" +# include "qemu_conf.h" +# include "qemu_event.h" +#include "qemu_process.h" + +#include "virerror.h" +#include "viralloc.h" +#include "virlog.h" +#include "virobject.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.qemu_event"); + +VIR_ENUM_IMPL(qemuMonitorEvent, + QEMU_EVENT_LAST, + "ACPI Event", "Balloon Change", "Block IO Error", + "Block Job Event", + "Block Write Threshold", "Device Deleted", + "Device Tray Moved", "Graphics", "Guest Panicked", + "Migration", "Migration pass", + "Nic RX Filter Changed", "Powerdown", "Reset", "Resume", + "RTC Change", "Shutdown", "Stop", + "Suspend", "Suspend To Disk", + "Virtual Serial Port Change", + "Wakeup", "Watchdog"); + +virQemuEventList* virQemuEventListInit(void) +{ +virQemuEventList *ev_list; +if (VIR_ALLOC(ev_list) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + "Unable to allocate virQemuEventList"); +return NULL; +} + +if (virMutexInit(&ev_list->lock) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot initialize mutex")); +VIR_FREE(ev_list); +return NULL; +} + +ev_list->head = NULL; +ev_list->last = NULL; + +return ev_list; +} diff --git a/src/qemu/qemu_event.h b/src/qemu/qemu_event.h new file mode 100644 index 000..9781795 --- /dev/null +++ b/src/qemu/qemu_event.h @@ -0,0 +1,224 @@ +/* + * qemu_event.h: interaction with QEMU JSON monitor event layer + * Carve out improved interactions with qemu. + * + * Copyright (C) 2017-2026 Nutanix, Inc. + * Copyright (C) 2017 Prerna Saxena + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOS
[libvirt] [[RFC] 4/8] Events: Allow monitor to "enqueue" events to a queue. Also introduce a framework of handlers for each event type, that can be called when the handler is running an event.
Signed-off-by: Prerna Saxena --- src/qemu/qemu_event.c| 11 + src/qemu/qemu_event.h| 8 + src/qemu/qemu_monitor.c | 592 +++- src/qemu/qemu_monitor.h | 80 +++-- src/qemu/qemu_monitor_json.c | 291 ++-- src/qemu/qemu_process.c | 789 +++ src/qemu/qemu_process.h | 2 + tests/qemumonitortestutils.c | 2 +- 8 files changed, 1273 insertions(+), 502 deletions(-) diff --git a/src/qemu/qemu_event.c b/src/qemu/qemu_event.c index d52fad2..beb309f 100644 --- a/src/qemu/qemu_event.c +++ b/src/qemu/qemu_event.c @@ -50,6 +50,7 @@ VIR_ENUM_IMPL(qemuMonitorEvent, "RTC Change", "Shutdown", "Stop", "Suspend", "Suspend To Disk", "Virtual Serial Port Change", + "Spice migrated", "Wakeup", "Watchdog"); virQemuEventList* virQemuEventListInit(void) @@ -302,3 +303,13 @@ void virDomainConsumeVMEvents(virDomainObjPtr vm, void *opaque) } return; } + +extern void qemuProcessEmitMonitorEvent(qemuEventPtr ev, void *opaque); + +void virEventRunHandler(qemuEventPtr ev, void *opaque) +{ +if (!ev) +return; + +return qemuProcessEmitMonitorEvent(ev, opaque); +} diff --git a/src/qemu/qemu_event.h b/src/qemu/qemu_event.h index 4173834..8552fd1 100644 --- a/src/qemu/qemu_event.h +++ b/src/qemu/qemu_event.h @@ -51,6 +51,7 @@ typedef enum { QEMU_EVENT_SUSPEND, QEMU_EVENT_SUSPEND_DISK, QEMU_EVENT_SERIAL_CHANGE, +QEMU_EVENT_SPICE_MIGRATED, QEMU_EVENT_WAKEUP, QEMU_EVENT_WATCHDOG, @@ -102,7 +103,12 @@ struct qemuEventTrayChangeData { int reason; }; +struct qemuEventShutdownData { +virTristateBool guest_initiated; +}; + struct qemuEventGuestPanicData { +void *info; }; struct qemuEventMigrationStatusData { @@ -159,6 +165,7 @@ struct _qemuEvent { struct qemuEventBlockThresholdData ev_threshold; struct qemuEventDeviceDeletedData ev_deviceDel; struct qemuEventTrayChangeData ev_tray; +struct qemuEventShutdownData ev_shutdown; struct qemuEventGuestPanicData ev_panic; struct qemuEventMigrationStatusData ev_migStatus; struct qemuEventMigrationPassData ev_migPass; @@ -219,5 +226,6 @@ int virQemuVmEventListInit(virDomainObjPtr vm); int virEnqueueVMEvent(virQemuEventList *qlist, qemuEventPtr ev); qemuEventPtr virDequeueVMEvent(virQemuEventList *qlist, virDomainObjPtr vm); void virEventWorkerScanQueue(void *dummy, void *opaque); +void virEventRunHandler(qemuEventPtr ev, void *opaque); void virDomainConsumeVMEvents(virDomainObjPtr vm, void *opaque); #endif diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7a26785..4e45cf9 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -34,6 +34,7 @@ #include "qemu_monitor_json.h" #include "qemu_domain.h" #include "qemu_process.h" +#include "qemu_event.h" #include "virerror.h" #include "viralloc.h" #include "virlog.h" @@ -1316,6 +1317,14 @@ qemuMonitorGetDiskSecret(qemuMonitorPtr mon, return ret; } +static int +qemuMonitorEnqueueEvent(qemuMonitorPtr mon, qemuEventPtr ev) +{ +int ret = -1; +QEMU_MONITOR_CALLBACK(mon, ret, domainEnqueueEvent, ev->vm, ev); + +return ret; +} int qemuMonitorEmitEvent(qemuMonitorPtr mon, const char *event, @@ -1332,90 +1341,189 @@ qemuMonitorEmitEvent(qemuMonitorPtr mon, const char *event, int -qemuMonitorEmitShutdown(qemuMonitorPtr mon, virTristateBool guest) +qemuMonitorEmitShutdown(qemuMonitorPtr mon, virTristateBool guest, +long long seconds, unsigned int micros) { int ret = -1; -VIR_DEBUG("mon=%p guest=%u", mon, guest); mon->willhangup = 1; +qemuEventPtr ev; + +if (VIR_ALLOC(ev) < 0){ +return ret; +} + +ev->ev_type = QEMU_EVENT_SHUTDOWN; +ev->vm = mon->vm; +ev->seconds = seconds; +ev->micros = micros; +ev->evData.ev_shutdown.guest_initiated = guest; -QEMU_MONITOR_CALLBACK(mon, ret, domainShutdown, mon->vm, guest); +VIR_DEBUG("Vm %s received shutdown event initiated by %u", +mon->vm->def->name, guest); +virObjectRef(ev->vm); +ret = qemuMonitorEnqueueEvent(mon, ev); return ret; } int -qemuMonitorEmitReset(qemuMonitorPtr mon) +qemuMonitorEmitReset(qemuMonitorPtr mon, long long seconds, unsigned int micros) { int ret = -1; -VIR_DEBUG("mon=%p", mon); +qemuEventPtr ev; + +if (VIR_ALLOC(ev) < 0){ +return ret; +} + +ev->ev_type = QEMU_EVENT_RESET; +ev->vm = mon->vm; +ev->seconds = seconds; +ev->micros = micros; -QEMU_MONITOR_CAL
[libvirt] [[RFC] 7/8] Fold back the 2-stage event implementation for a few events : Watchdog, Monitor EOF, Serial changed, Guest panic, Nic RX filter changed .. into single level.
Also, the enqueuing of a new event now triggers virEventWorkerScanQueue() Signed-off-by: Prerna Saxena --- src/qemu/qemu_driver.c | 61 ++-- src/qemu/qemu_process.c | 121 +++- 2 files changed, 29 insertions(+), 153 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9d495fb..881f253 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -138,8 +138,6 @@ VIR_LOG_INIT("qemu.qemu_driver"); #define QEMU_NB_BANDWIDTH_PARAM 7 -static void qemuProcessEventHandler(void *data, void *opaque); - static int qemuStateCleanup(void); static int qemuDomainObjStart(virConnectPtr conn, @@ -936,7 +934,9 @@ qemuStateInitialize(bool privileged, qemuProcessReconnectAll(conn, qemu_driver); -qemu_driver->workerPool = virThreadPoolNew(0, 1, 0, qemuProcessEventHandler, qemu_driver); +qemu_driver->workerPool = virThreadPoolNew(0, 1, 0, virEventWorkerScanQueue, + qemu_driver); + if (!qemu_driver->workerPool) goto error; @@ -3645,61 +3645,6 @@ qemuDomainScreenshot(virDomainPtr dom, } - - - - - - - - -static void qemuProcessEventHandler(void *data, void *opaque) -{ -struct qemuProcessEvent *processEvent = data; -virDomainObjPtr vm = processEvent->vm; -virQEMUDriverPtr driver = opaque; - -VIR_DEBUG("vm=%p, event=%d", vm, processEvent->eventType); - -virObjectLock(vm); - -switch (processEvent->eventType) { -case QEMU_PROCESS_EVENT_WATCHDOG: -processWatchdogEvent(driver, vm, processEvent->action); -break; -case QEMU_PROCESS_EVENT_GUESTPANIC: -processGuestPanicEvent(driver, vm, processEvent->action, - processEvent->data); -break; -case QEMU_PROCESS_EVENT_DEVICE_DELETED: -processDeviceDeletedEvent(driver, vm, processEvent->data); -break; -case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED: -processNicRxFilterChangedEvent(driver, vm, processEvent->data); -break; -case QEMU_PROCESS_EVENT_SERIAL_CHANGED: -processSerialChangedEvent(driver, vm, processEvent->data, - processEvent->action); -break; -case QEMU_PROCESS_EVENT_BLOCK_JOB: -processBlockJobEvent(driver, vm, - processEvent->data, - processEvent->action, - processEvent->status); -break; -case QEMU_PROCESS_EVENT_MONITOR_EOF: -processMonitorEOFEvent(driver, vm); -break; -case QEMU_PROCESS_EVENT_LAST: -break; -} - -virDomainConsumeVMEvents(vm, driver); -virDomainObjEndAPI(&vm); -VIR_FREE(processEvent); -} - - static int qemuDomainSetVcpusAgent(virDomainObjPtr vm, unsigned int nvcpus) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d2b5fe8..f9270e0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -962,21 +962,11 @@ qemuProcessEventHandleWatchdog1(qemuEventPtr ev, } if (vm->def->watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) { -struct qemuProcessEvent *processEvent; -if (VIR_ALLOC(processEvent) == 0) { -processEvent->eventType = QEMU_PROCESS_EVENT_WATCHDOG; -processEvent->action = VIR_DOMAIN_WATCHDOG_ACTION_DUMP; -processEvent->vm = vm; -/* Hold an extra reference because we can't allow 'vm' to be - * deleted before handling watchdog event is finished. - */ -virObjectRef(vm); -if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { -if (!virObjectUnref(vm)) -vm = NULL; -VIR_FREE(processEvent); -} -} +/* Hold an extra reference because we can't allow 'vm' to be + * deleted before handling watchdog event is finished.*/ +virObjectRef(vm); +processWatchdogEvent(driver, vm, VIR_DOMAIN_WATCHDOG_ACTION_DUMP); +virObjectUnref(vm); } qemuDomainEventQueue(driver, watchdogEvent); @@ -1068,12 +1058,10 @@ qemuProcessEventHandleBlockJob(qemuEventPtr ev, void *opaque) { virQEMUDriverPtr driver = opaque; -struct qemuProcessEvent *processEvent = NULL; virDomainDiskDefPtr disk; qemuDomainDiskPrivatePtr diskPriv; -char *data = NULL; virDomainObjPtr vm; -const char *diskAlias; +char *diskAlias = NULL; int type, status; if (!ev) @@ -1082,7 +1070,7 @@ qemuProcessEventHandleBlockJob(qemuEventPtr ev, if (!ev->vm) { VIR_WARN("Unable to locate VM, dropping Block Job event"); -goto cleanup; +
[libvirt] [[RFC] 5/8] Events: Plumb event handling calls before a domain's APIs complete.
Signed-off-by: Prerna Saxena --- src/qemu/qemu_driver.c | 131 +++-- 1 file changed, 128 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8a005d0..b249347 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1900,6 +1900,7 @@ static int qemuDomainSuspend(virDomainPtr dom) qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); qemuDomainEventQueue(driver, event); @@ -1967,6 +1968,7 @@ static int qemuDomainResume(virDomainPtr dom) qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); qemuDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -2057,6 +2059,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); return ret; } @@ -2159,6 +2162,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); return ret; } @@ -2206,6 +2210,7 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags) qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); return ret; } @@ -2294,6 +2299,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); qemuDomainEventQueue(driver, event); return ret; @@ -2307,6 +2313,7 @@ qemuDomainDestroy(virDomainPtr dom) static char *qemuDomainGetOSType(virDomainPtr dom) { virDomainObjPtr vm; +virQEMUDriverPtr driver = dom->conn->privateData; char *type = NULL; if (!(vm = qemuDomObjFromDomain(dom))) @@ -2318,6 +2325,7 @@ static char *qemuDomainGetOSType(virDomainPtr dom) { ignore_value(VIR_STRDUP(type, virDomainOSTypeToString(vm->def->os.type))); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); return type; } @@ -2328,6 +2336,7 @@ qemuDomainGetMaxMemory(virDomainPtr dom) { virDomainObjPtr vm; unsigned long long ret = 0; +virQEMUDriverPtr driver = dom->conn->privateData; if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -2338,6 +2347,7 @@ qemuDomainGetMaxMemory(virDomainPtr dom) ret = virDomainDefGetMemoryTotal(vm->def); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); return ret; } @@ -2455,6 +2465,7 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; @@ -2543,6 +2554,7 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; @@ -2583,6 +2595,7 @@ static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); return ret; } @@ -2646,6 +2659,7 @@ static int qemuDomainSendKey(virDomainPtr domain, qemuDomainObjEndJob(driver, vm); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); return ret; } @@ -2702,6 +2716,7 @@ qemuDomainGetInfo(virDomainPtr dom, ret = 0; cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); return ret; } @@ -2739,6 +2754,7 @@ qemuDomainGetControlInfo(virDomainPtr dom, virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; int ret = -1; +virQEMUDriverPtr driver = dom->conn->privateData; virCheckFlags(0, -1); @@ -2788,6 +2804,7 @@ qemuDomainGetControlInfo(virDomainPtr dom, ret = 0; cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); return ret; } @@ -3577,6 +3594,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, compressedpath, dxml, flags); cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); VIR_FREE(compressedpath); virObjectUnref(cfg); @@ -3653,6 +3671,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) vm->hasManagedSave = true; cleanup: +virDomainConsumeVMEvents(vm, driver); virDomainObjEndAPI(&vm); VIR_FREE(name); VIR_FREE(compressedpath); @@ -3689,7 +3708,7 @@ qemuDoma
[libvirt] [[RFC] 8/8] Initialize the per-VM event queues in context of domain init.
Signed-off-by: Prerna Saxena --- src/qemu/qemu_driver.c | 20 src/qemu/qemu_event.c | 2 ++ 2 files changed, 22 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 881f253..e4b6d06 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1781,6 +1781,11 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; + +if (virQemuVmEventListInit(vm) < 0) { +virDomainObjListRemove(driver->domains, vm); +goto cleanup; +} virObjectRef(vm); def = NULL; @@ -5531,6 +5536,11 @@ qemuDomainRestoreFlags(virConnectPtr conn, VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; + +if (virQemuVmEventListInit(vm) < 0) { +virDomainObjListRemove(driver->domains, vm); +goto cleanup; +} virObjectRef(vm); def = NULL; @@ -6208,6 +6218,11 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, 0, &oldDef))) goto cleanup; +if (virQemuVmEventListInit(vm) < 0) { +virDomainObjListRemove(driver->domains, vm); +goto cleanup; +} + virObjectRef(vm); def = NULL; if (qemuDomainHasBlockjob(vm, true)) { @@ -15073,6 +15088,11 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn, NULL))) goto cleanup; +if (virQemuVmEventListInit(vm) < 0) { +virDomainObjListRemove(driver->domains, vm); +goto cleanup; +} + virObjectRef(vm); def = NULL; diff --git a/src/qemu/qemu_event.c b/src/qemu/qemu_event.c index beb309f..00a06ee 100644 --- a/src/qemu/qemu_event.c +++ b/src/qemu/qemu_event.c @@ -145,6 +145,8 @@ int virEnqueueVMEvent(virQemuEventList *qlist, qemuEventPtr ev) /* Insert into per-Vm queue */ vmq = ev->vm->vmq; +if (!vmq) +goto error; virMutexLock(&(vmq->lock)); if (vmq->last) { -- 2.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [[RFC] 6/8] Code refactor: Move helper functions of doCoreDump*, syncNicRxFilter*, and qemuOpenFile* to qemu_process.[ch]
Signed-off-by: Prerna Saxena --- src/qemu/qemu_driver.c | 1161 --- src/qemu/qemu_process.c | 1133 + src/qemu/qemu_process.h | 86 3 files changed, 1219 insertions(+), 1161 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b249347..9d495fb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -151,11 +151,6 @@ static int qemuDomainObjStart(virConnectPtr conn, static int qemuDomainManagedSaveLoad(virDomainObjPtr vm, void *opaque); -static int qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, - bool dynamicOwnership, - const char *path, int oflags, - bool *needUnlink, bool *bypassSecurityDriver); - static int qemuGetDHCPInterfaces(virDomainPtr dom, virDomainObjPtr vm, virDomainInterfacePtr **ifaces); @@ -2819,38 +2814,6 @@ qemuDomainGetControlInfo(virDomainPtr dom, verify(sizeof(QEMU_SAVE_MAGIC) == sizeof(QEMU_SAVE_PARTIAL)); -typedef enum { -QEMU_SAVE_FORMAT_RAW = 0, -QEMU_SAVE_FORMAT_GZIP = 1, -QEMU_SAVE_FORMAT_BZIP2 = 2, -/* - * Deprecated by xz and never used as part of a release - * QEMU_SAVE_FORMAT_LZMA - */ -QEMU_SAVE_FORMAT_XZ = 3, -QEMU_SAVE_FORMAT_LZOP = 4, -/* Note: add new members only at the end. - These values are used in the on-disk format. - Do not change or re-use numbers. */ - -QEMU_SAVE_FORMAT_LAST -} virQEMUSaveFormat; - -VIR_ENUM_DECL(qemuSaveCompression) -VIR_ENUM_IMPL(qemuSaveCompression, QEMU_SAVE_FORMAT_LAST, - "raw", - "gzip", - "bzip2", - "xz", - "lzop") - -VIR_ENUM_DECL(qemuDumpFormat) -VIR_ENUM_IMPL(qemuDumpFormat, VIR_DOMAIN_CORE_DUMP_FORMAT_LAST, - "elf", - "kdump-zlib", - "kdump-lzo", - "kdump-snappy") - typedef struct _virQEMUSaveHeader virQEMUSaveHeader; typedef virQEMUSaveHeader *virQEMUSaveHeaderPtr; struct _virQEMUSaveHeader { @@ -3062,214 +3025,6 @@ qemuCompressGetCommand(virQEMUSaveFormat compression) return ret; } -/** - * qemuOpenFile: - * @driver: driver object - * @vm: domain object - * @path: path to file to open - * @oflags: flags for opening/creation of the file - * @needUnlink: set to true if file was created by this function - * @bypassSecurityDriver: optional pointer to a boolean that will be set to true - *if security driver operations are pointless (due to - *NFS mount) - * - * Internal function to properly create or open existing files, with - * ownership affected by qemu driver setup and domain DAC label. - * - * Returns the file descriptor on success and negative errno on failure. - * - * This function should not be used on storage sources. Use - * qemuDomainStorageFileInit and storage driver APIs if possible. - **/ -static int -qemuOpenFile(virQEMUDriverPtr driver, - virDomainObjPtr vm, - const char *path, - int oflags, - bool *needUnlink, - bool *bypassSecurityDriver) -{ -int ret = -1; -virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); -uid_t user = cfg->user; -gid_t group = cfg->group; -bool dynamicOwnership = cfg->dynamicOwnership; -virSecurityLabelDefPtr seclabel; - -virObjectUnref(cfg); - -/* TODO: Take imagelabel into account? */ -if (vm && -(seclabel = virDomainDefGetSecurityLabelDef(vm->def, "dac")) != NULL && -seclabel->label != NULL && -(virParseOwnershipIds(seclabel->label, &user, &group) < 0)) -goto cleanup; - -ret = qemuOpenFileAs(user, group, dynamicOwnership, - path, oflags, needUnlink, bypassSecurityDriver); - - cleanup: -return ret; -} - -static int -qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, - bool dynamicOwnership, - const char *path, int oflags, - bool *needUnlink, bool *bypassSecurityDriver) -{ -struct stat sb; -bool is_reg = true; -bool need_unlink = false; -bool bypass_security = false; -unsigned int vfoflags = 0; -int fd = -1; -int path_shared = virFileIsSharedFS(path); -uid_t uid = geteuid(); -gid_t gid = getegid(); - -/* path might be a pre-existing block dev, in which case - * we need to skip the create step, and also avoid unlink - * in the failure case */ -if (oflags & O_CREAT) { -need_unlink = true; - -/* Don't force chown on network-shared FS - * as it is likely to fail. */ -if (path_shared <=
[libvirt] [[RFC] 1/8] Introduce virObjectTrylock()
This is a wrapper function that: (1) Attempts to take a lock on the object. (2) gracefully returns if the object is already locked. Signed-off-by: Prerna Saxena --- src/libvirt_private.syms | 1 + src/util/virobject.c | 26 ++ src/util/virobject.h | 4 src/util/virthread.c | 5 + src/util/virthread.h | 1 + 5 files changed, 37 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9243c55..c0ab8b5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2362,6 +2362,7 @@ virObjectRWLockableNew; virObjectRWLockRead; virObjectRWLockWrite; virObjectRWUnlock; +virObjectTrylock; virObjectUnlock; virObjectUnref; diff --git a/src/util/virobject.c b/src/util/virobject.c index cfa821c..796ea06 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -495,6 +495,32 @@ virObjectRWLockWrite(void *anyobj) /** + * virObjectTrylock: + * @anyobj: any instance of virObjectLockable or virObjectRWLockable + * + * Attempt to acquire a lock on @anyobj. The lock must be released by + * virObjectUnlock. + * Returns: + *0: If the lock was successfully taken. + *errno : Indicates error. + * + * The caller is expected to have acquired a reference + * on the object before locking it (eg virObjectRef). + * The object must be unlocked before releasing this + * reference. + */ +int +virObjectTrylock(void *anyobj) +{ +virObjectLockablePtr obj = virObjectGetLockableObj(anyobj); + +if (!obj) +return -1; + +return virMutexTrylock(&obj->lock); +} + +/** * virObjectUnlock: * @anyobj: any instance of virObjectLockable * diff --git a/src/util/virobject.h b/src/util/virobject.h index ac6cf22..402ea32 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -124,6 +124,10 @@ void virObjectLock(void *lockableobj) ATTRIBUTE_NONNULL(1); +int +virObjectTrylock(void *lockableobj) +ATTRIBUTE_NONNULL(1); + void virObjectRWLockRead(void *lockableobj) ATTRIBUTE_NONNULL(1); diff --git a/src/util/virthread.c b/src/util/virthread.c index 6c49515..07b7a3f 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -89,6 +89,11 @@ void virMutexLock(virMutexPtr m) pthread_mutex_lock(&m->lock); } +int virMutexTrylock(virMutexPtr m) +{ +return pthread_mutex_trylock(&m->lock); +} + void virMutexUnlock(virMutexPtr m) { pthread_mutex_unlock(&m->lock); diff --git a/src/util/virthread.h b/src/util/virthread.h index e466d9b..8e3da2c 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -132,6 +132,7 @@ int virMutexInitRecursive(virMutexPtr m) ATTRIBUTE_RETURN_CHECK; void virMutexDestroy(virMutexPtr m); void virMutexLock(virMutexPtr m); +int virMutexTrylock(virMutexPtr m); void virMutexUnlock(virMutexPtr m); -- 2.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [[RFC] 3/8] Setup global and per-VM event queues. Also initialize per-VM queues when libvirt reconnects to an existing VM.
Signed-off-by: Prerna Saxena --- src/conf/domain_conf.h | 3 + src/qemu/qemu_conf.h| 4 + src/qemu/qemu_driver.c | 9 ++ src/qemu/qemu_event.c | 229 src/qemu/qemu_event.h | 1 - src/qemu/qemu_process.c | 2 + 6 files changed, 247 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a42efcf..7fe38e7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2496,6 +2496,9 @@ struct _virDomainObj { unsigned long long original_memlock; /* Original RLIMIT_MEMLOCK, zero if no * restore will be required later */ + +/* Pointer to per-VM Event Queue */ +void *vmq; }; typedef bool (*virDomainObjListACLFilter)(virConnectPtr conn, diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 13b6f81..e63dc98 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -33,6 +33,7 @@ # include "domain_conf.h" # include "snapshot_conf.h" # include "domain_event.h" +# include "qemu_event.h" # include "virthread.h" # include "security/security_manager.h" # include "virpci.h" @@ -235,6 +236,9 @@ struct _virQEMUDriver { /* Immutable pointer, self-locking APIs */ virDomainObjListPtr domains; +/* Immutable pointer, contains Qemu Driver Event List */ +virQemuEventList *ev_list; + /* Immutable pointer */ char *qemuImgBinary; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7c6f167..8a005d0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -52,6 +52,7 @@ #include "qemu_command.h" #include "qemu_parse_command.h" #include "qemu_cgroup.h" +#include "qemu_event.h" #include "qemu_hostdev.h" #include "qemu_hotplug.h" #include "qemu_monitor.h" @@ -650,6 +651,14 @@ qemuStateInitialize(bool privileged, if (!(qemu_driver->domains = virDomainObjListNew())) goto error; +/* Init domain Async QMP events */ +qemu_driver->ev_list = virQemuEventListInit(); +if (!qemu_driver->ev_list) { +virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize QMP event queues")); +goto error; +} + /* Init domain events */ qemu_driver->domainEventState = virObjectEventStateNew(); if (!qemu_driver->domainEventState) diff --git a/src/qemu/qemu_event.c b/src/qemu/qemu_event.c index e27ea0d..d52fad2 100644 --- a/src/qemu/qemu_event.c +++ b/src/qemu/qemu_event.c @@ -73,3 +73,232 @@ virQemuEventList* virQemuEventListInit(void) return ev_list; } + +int virQemuVmEventListInit(virDomainObjPtr vm) +{ +virQemuVmEventQueue* vmq; +if (!vm) +return -1; + +if (VIR_ALLOC(vmq) < 0) +return -1; + +vmq->last = NULL; +vmq->head = NULL; + +if (!virMutexInit(&vmq->lock)) { +vm->vmq = vmq; +return 0; +} +return -1; +} +/** + * virEnqueueVMEvent() + * Adds a new event to: + * - Global event queue + * - the event queue for this VM + * + * Return : 0 (success) + * -1 (failure) + */ +int virEnqueueVMEvent(virQemuEventList *qlist, qemuEventPtr ev) +{ +struct _qemuGlobalEventListElement *globalEntry; +virQemuVmEventQueue *vmq; +struct _qemuVmEventQueueElement *vmq_entry; + +if (!qlist || !ev || !ev->vm || !ev->vm->vmq) { +virReportError(VIR_ERR_INTERNAL_ERROR, + "No queue list instantiated." + "Dropping event %d for Vm %s", + ev->ev_type, ev->vm->def->name); +goto error; +} + +if (VIR_ALLOC(globalEntry) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + "Allocation error." + "Dropping event %d for Vm %s", + ev->ev_type, ev->vm->def->name); +goto error; +} + +if (VIR_ALLOC(vmq_entry) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + "Allocation error." + "Dropping event %d for Vm %s", + ev->ev_type, ev->vm->def->name); +free(globalEntry); +goto error; +} + +vmq_entry->ev = ev; +vmq_entry->next = NULL; + +virObjectRef(ev->vm); +globalEntry->vm = ev->vm; +globalEntry->next = NULL; +globalEntry->prev = NULL; +/* Note that this order needs to be maintained + * for dequeue too else ABBA deadlocks will happen */ + +/* Insert into per-Vm queue */ +vmq = ev->vm->vmq; + +virMutexLock(&(vmq->lock)); +if (vmq->last) { +vmq->last->next = vmq_entry; +
[libvirt] [[RFC] 0/8] Implement async QEMU event handling in libvirtd.
As noted in https://www.redhat.com/archives/libvir-list/2017-May/msg00016.html libvirt-QEMU driver handles all async events from the main loop. Each event handling needs the per-VM lock to make forward progress. In the case where an async event is received for the same VM which has an RPC running, the main loop is held up contending for the same lock. This impacts scalability, and should be addressed on priority. Note that libvirt does have a 2-step deferred handling for a few event categories, but (1) That is insufficient since blockign happens before the handler could disambiguate which one needs to be posted to this other queue. (2) There needs to be homogeniety. The current series builds a framework for recording and handling VM events. It initializes per-VM event queue, and a global event queue pointing to events from all the VMs. Event handling is staggered in 2 stages: - When an event is received, it is enqueued in the per-VM queue as well as the global queues. - The global queue is built into the QEMU Driver as a threadpool (currently with a single thread). - Enqueuing of a new event triggers the global event worker thread, which then attempts to take a lock for this event's VM. - If the lock is available, the event worker runs the function handling this event type. Once done, it dequeues this event from the global as well as per-VM queues. - If the lock is unavailable(ie taken by RPC thread), the event worker thread leaves this as-is and picks up the next event. - Once the RPC thread completes, it looks for events pertaining to the VM in the per-VM event queue. It then processes the events serially (holding the VM lock) until there are no more events remaining for this VM. At this point, the per-VM lock is relinquished. Patch Series status: Strictly RFC only. No compilation issues. I have not had a chance to (stress) test it after rebase to latest master. Note that documentation and test coverage is TBD, since a few open points remain. Known issues/ caveats: - RPC handling time will become non-deterministic. - An event will only be "notified" to a client once the RPC for same VM completes. - Needs careful consideration in all cases where a QMP event is used to "signal" an RPC thread, else will deadlock. Will be happy to drive more discussion in the community and completely implement it. Prerna Saxena (8): Introduce virObjectTrylock() QEMU Event handling: Introduce async event helpers in qemu_event.[ch] Setup global and per-VM event queues. Also initialize per-VM queues when libvirt reconnects to an existing VM. Events: Allow monitor to "enqueue" events to a queue. Also introduce a framework of handlers for each event type, that can be called when the handler is running an event. Events: Plumb event handling calls before a domain's APIs complete. Code refactor: Move helper functions of doCoreDump*, syncNicRxFilter*, and qemuOpenFile* to qemu_process.[ch] Fold back the 2-stage event implementation for a few events : Watchdog, Monitor EOF, Serial changed, Guest panic, Nic RX filter changed .. into single level. Initialize the per-VM event queues in context of domain init. src/Makefile.am |1 + src/conf/domain_conf.h |3 + src/libvirt_private.syms |1 + src/qemu/qemu_conf.h |4 + src/qemu/qemu_driver.c | 1710 +++ src/qemu/qemu_event.c| 317 +++ src/qemu/qemu_event.h| 231 + src/qemu/qemu_monitor.c | 592 ++-- src/qemu/qemu_monitor.h | 80 +- src/qemu/qemu_monitor_json.c | 291 +++--- src/qemu/qemu_process.c | 2031 ++ src/qemu/qemu_process.h | 88 ++ src/util/virobject.c | 26 + src/util/virobject.h |4 + src/util/virthread.c |5 + src/util/virthread.h |1 + tests/qemumonitortestutils.c |2 +- 17 files changed, 3411 insertions(+), 1976 deletions(-) create mode 100644 src/qemu/qemu_event.c create mode 100644 src/qemu/qemu_event.h -- 2.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] Debug: Report VM errors more concisely.
Current logs: error : qemuProcessFindDomainDiskByAlias:411 : internal error: no disk found with alias ide0-0-0 There is no way to find which VM was seeing this error. Makes debugging very hard, and the message itself is no good. Signed-off-by: Prerna Saxena --- src/qemu/qemu_process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e450d06..0a052e8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -367,8 +367,8 @@ qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, } virReportError(VIR_ERR_INTERNAL_ERROR, - _("no disk found with alias %s"), - alias); + _("VM %s: no disk found with alias %s"), + vm->def->name, alias); return NULL; } -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] Debug: Add DEBUG messages for when a client has opened/closed connection.
While tracing connections from a remote client, it helps to keep track of the connection lifecycle. Messages such as the following : error : virNetSocketReadWire:1574 : End of file while reading data: Input/output error are rather unhelpful. They do not indicate if the client had earlier asked for connection closure via libvirt API. This patch introduces messages to annotate when a client connected/disconnected. Signed-off-by: Prerna Saxena --- src/rpc/virnetserverclient.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 85857bc..24c9c33 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -964,8 +964,11 @@ void virNetServerClientClose(virNetServerClientPtr client) virNetServerClientCloseFunc cf; virKeepAlivePtr ka; +VIR_DEBUG("Free'ing up resources for client=%p sock=%d", client, + virNetServerClientGetFD(client)); + virObjectLock(client); -VIR_DEBUG("client=%p", client); + if (!client->sock) { virObjectUnlock(client); return; @@ -1039,10 +1042,14 @@ void virNetServerClientDelayedClose(virNetServerClientPtr client) virObjectLock(client); client->delayedClose = true; virObjectUnlock(client); +VIR_DEBUG("Client=%p sock=%d requested closure of connection.", + client, virNetServerClientGetFD(client)); } void virNetServerClientImmediateClose(virNetServerClientPtr client) { +VIR_DEBUG("Client %p sock %d closed the connection", client, +virNetServerClientGetFD(client)); virObjectLock(client); client->wantClose = true; virObjectUnlock(client); @@ -1151,6 +1158,7 @@ static void virNetServerClientDispatchRead(virNetServerClientPtr client) if (client->rx->nfds == 0) { if (virNetServerClientRead(client) < 0) { client->wantClose = true; +VIR_WARN("Client=%p sock=%p closed connection", client, client->sock); return; /* Error */ } } -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/2] Augment debug messages.
This is a v2 of the previous set of debug enhancements posted at : https://www.redhat.com/archives/libvir-list/2017-March/msg00959.html Changelog: - 1) Patch 1/3 : virNetSocketGetFD() is reverted to its old state, and only new DEBUG messages are introduced. 2) Patch 2/3 : Dropped, per list suggestions. 3) Patch 3/3 : Same as v1 Prerna Saxena (2): Debug: Add DEBUG messages for when a client has opened/closed connection. Debug: Report VM errors more concisely. src/qemu/qemu_process.c | 4 ++-- src/rpc/virnetserverclient.c | 10 +- 2 files changed, 11 insertions(+), 3 deletions(-) -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] Debug: Report VM errors more concisely.
Current logs: error : qemuProcessFindDomainDiskByAlias:411 : internal error: no disk found with alias ide0-0-0 There is no way to find which VM was seeing this error. Makes debugging very hard, and the message itself is no good. Signed-off-by: Prerna Saxena --- src/qemu/qemu_process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ec0e36d..9f5bc3a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -366,8 +366,8 @@ qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, } virReportError(VIR_ERR_INTERNAL_ERROR, - _("no disk found with alias %s"), - alias); + _("VM %s: no disk found with alias %s"), + vm->def->name, alias); return NULL; } -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Debug: Add WARN'ing messages for when a client has opened/closed connection.
While tracing connections from a remote client, it helps to keep track of the connection lifecycle. Messages such as the following : error : virNetSocketReadWire:1574 : End of file while reading data: Input/output error are rather unhelpful. They do not indicate if the client had earlier asked for connection closure via libvirt API. This patch introduces messages to annotate when a client connected/disconnected. Signed-off-by: Prerna Saxena --- src/rpc/virnetserverclient.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 85857bc..a77feaa 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -678,14 +678,19 @@ int virNetServerClientGetTLSKeySize(virNetServerClientPtr client) return size; } #endif - +/* + * This mostly just needs to publish the client socket FD to logs. + * It does not necessarily need a lock, or will add lock contention problems. + * Replace the lock with a reference counting mechanism, to prevent the client + * object from being deallocated while this is being run + */ int virNetServerClientGetFD(virNetServerClientPtr client) { int fd = -1; -virObjectLock(client); +virObjectRef(client); if (client->sock) fd = virNetSocketGetFD(client->sock); -virObjectUnlock(client); +virObjectUnref(client); return fd; } @@ -965,7 +970,9 @@ void virNetServerClientClose(virNetServerClientPtr client) virKeepAlivePtr ka; virObjectLock(client); -VIR_DEBUG("client=%p", client); +VIR_WARN("Free'ing up resources for client=%p sock=%d", client, + virNetServerClientGetFD(client)); + if (!client->sock) { virObjectUnlock(client); return; @@ -1039,6 +1046,8 @@ void virNetServerClientDelayedClose(virNetServerClientPtr client) virObjectLock(client); client->delayedClose = true; virObjectUnlock(client); +VIR_WARN("Client=%p sock=%d requested closure of connection.", + client, virNetServerClientGetFD(client)); } void virNetServerClientImmediateClose(virNetServerClientPtr client) @@ -1151,6 +1160,7 @@ static void virNetServerClientDispatchRead(virNetServerClientPtr client) if (client->rx->nfds == 0) { if (virNetServerClientRead(client) < 0) { client->wantClose = true; +VIR_WARN("Client=%p sock=%p closed connection", client, client->sock); return; /* Error */ } } -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Debug: Remove unnecessary errors reported while parsing non-existent sysfs files.
Sample from current logs: error : virFileReadAll:1290 : Failed to open file '/sys/class/net/tap3/operstate': No such file or directory error : virNetDevGetLinkInfo:1895 : unable to read: /sys/class/net/tap3/operstate: No such file or directory These have no useful data point and are redundant. Signed-off-by: Prerna Saxena --- src/util/virnetdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index d123248..3e2f962 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1874,7 +1874,7 @@ virNetDevGetLinkInfo(const char *ifname, if (virNetDevSysfsFile(&path, ifname, "operstate") < 0) goto cleanup; -if (virFileReadAll(path, 1024, &buf) < 0) { +if (virFileReadAllQuiet(path, 1024, &buf) < 0 && errno != ENOENT) { virReportSystemError(errno, _("unable to read: %s"), path); -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Debug: Improve log messages.
Libvirt logs include many snippets for debugging daemon state, but some of those messages are either missing vital information or end up logging "errors" for normal operating conditions as well. This series improves log messages, also adding additional WARN'ings for connection instantiation and closure. Prerna Saxena (3): Debug: Add WARN'ing messages for when a client has opened/closed connection. Debug: Remove unnecessary errors reported while parsing non-existent sysfs files. Debug: Report VM errors more concisely. src/qemu/qemu_process.c | 4 ++-- src/rpc/virnetserverclient.c | 18 ++ src/util/virnetdev.c | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Qemu : Do not distinguish a 'hangup' from an 'eof'
An errno=ECONNRESET received on a monitor socket reflects that the guest may have closed the socket. Today, we just mark it as a 'hangup' and do not trigger the eof callback. I've been looking at a slew of such messages in libvirt logs. If the monitor socket indicated an ECONNRESET, it would possibly make sense to mark the socket closed so that the "eof" callback may then be invoked. Is there a subtle corner case I'm missing here ? Signed-off-by: Prerna Saxena --- src/qemu/qemu_monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a5e14b2..dcafa5a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -690,10 +690,11 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) if (events & VIR_EVENT_HANDLE_HANGUP) { hangup = true; +eof = true; if (!error) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("End of file from qemu monitor")); -eof = true; + events &= ~VIR_EVENT_HANDLE_HANGUP; } } -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Debug: Record the VM PID in per-VM logs.
For certain situations such as Out-of-memory scenarios, it is helpful to record the QEMU PID in the per-VM logs, so that correlation with kernel-reported events is easier. While this is already recorded in libvirt daemon logs as a DEBUG message, I believe it merits more attention :-) Hence introducing it in per-VM log. Signed-off-by: Prerna Saxena --- src/qemu/qemu_process.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4adb14e..2d3b0f5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4983,6 +4983,7 @@ qemuProcessLaunch(virConnectPtr conn, int ret = -1; int rv; int logfile = -1; +char *msg_buffer = NULL; qemuDomainLogContextPtr logCtxt = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCommandPtr cmd = NULL; @@ -5095,8 +5096,14 @@ qemuProcessLaunch(virConnectPtr conn, _("Domain %s didn't show up"), vm->def->name); rv = -1; } + VIR_DEBUG("QEMU vm=%p name=%s running with pid=%llu", vm, vm->def->name, (unsigned long long)vm->pid); +ignore_value(virAsprintf(&msg_buffer, +"QEMU vm=%p name=%s running with pid=%llu", +vm, vm->def->name, (unsigned long long)vm->pid)); +qemuLogOperation(vm, msg_buffer, NULL, logCtxt); +virFree(msg_buffer); } else { VIR_DEBUG("QEMU vm=%p name=%s failed to spawn", vm, vm->def->name); -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/2] Storage: Introduce shadow vol for refresh while the main vol builds.
On Tuesday 30 June 2015 06:20 PM, Ján Tomko wrote: > On Fri, Jun 26, 2015 at 05:05:11PM +0530, Prerna Saxena wrote: >> Libvirt periodically refreshes all volumes in a storage pool, including >> the volumes being cloned. >> While cloning a storage volume from parent, we drop pool locks. Subsequent >> volume refresh sometimes changes allocation for an ongoing copy, and leads >> to corrupt images. >> Fix: Introduce a shadow volume that isolates the volume object under refresh >> from the base which has a copy ongoing. >> >> Signed-off-by: Prerna Saxena >> --- >> src/storage/storage_driver.c | 14 -- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> > ACK > >> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c >> index 57060ab..41de8df 100644 >> --- a/src/storage/storage_driver.c >> +++ b/src/storage/storage_driver.c >> @@ -1898,7 +1898,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, >> { >> virStoragePoolObjPtr pool, origpool = NULL; >> virStorageBackendPtr backend; >> -virStorageVolDefPtr origvol = NULL, newvol = NULL; >> +virStorageVolDefPtr origvol = NULL, newvol = NULL, shadowvol = NULL; >> virStorageVolPtr ret = NULL, volobj = NULL; >> unsigned long long allocation; >> int buildret; >> @@ -2010,6 +2010,15 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, >> if (backend->createVol(obj->conn, pool, newvol) < 0) >> goto cleanup; >> >> +/* Make a shallow copy of the 'defined' volume definition, since the >> + * original allocation value will change as the user polls 'info', >> + * but we only need the initial requested values >> + */ >> +if (VIR_ALLOC(shadowvol) < 0) >> +goto cleanup; >> + >> +memcpy(shadowvol, newvol, sizeof(*newvol)); >> + >> pool->volumes.objs[pool->volumes.count++] = newvol; >> volobj = virGetStorageVol(obj->conn, pool->def->name, newvol->name, >>newvol->key, NULL, NULL); >> @@ -2029,7 +2038,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, >> virStoragePoolObjUnlock(origpool); >> } >> >> -buildret = backend->buildVolFrom(obj->conn, pool, newvol, origvol, >> flags); >> +buildret = backend->buildVolFrom(obj->conn, pool, shadowvol, origvol, >> flags); >> > newvol->target.allocation should also use the value from shadowvol. > I have made the adjustment and pushed the patch. > > Jan Hi Jan, Thanks. Could you also take a look at the second patch in this series ? Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/2] Storage : Fix cloning of raw, sparse volumes
When virsh vol-clone is attempted on a raw file where capacity > allocation, the resulting cloned volume has a size that matches the virtual-size of the parent; in place of matching its actual, disk size. This patch fixes the cloned disk to have same _allocated_size_ as the parent file from which it was cloned. Ref: http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html Also fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1130739 Signed-off-by: Prerna Saxena --- src/storage/storage_backend.c | 23 ++- src/storage/storage_driver.c | 5 - 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index ce59f63..492b344 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -342,7 +342,7 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } -remain = vol->target.allocation; +remain = vol->target.capacity; if (inputvol) { int res = virStorageBackendCopyToFD(vol, inputvol, @@ -397,7 +397,8 @@ createRawFile(int fd, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, bool reflink_copy) { -bool need_alloc = true; +bool want_sparse = (inputvol && + (inputvol->target.capacity > vol->target.allocation)); int ret = 0; unsigned long long remain; @@ -420,10 +421,9 @@ createRawFile(int fd, virStorageVolDefPtr vol, * to writing zeroes block by block in case fallocate isn't * available, and since we're going to copy data from another * file it doesn't make sense to write the file twice. */ -if (vol->target.allocation) { -if (fallocate(fd, 0, 0, vol->target.allocation) == 0) { -need_alloc = false; -} else if (errno != ENOSYS && errno != EOPNOTSUPP) { +if (vol->target.allocation && !want_sparse) { +if ((fallocate(fd, 0, 0, vol->target.allocation) != 0) && + (errno != ENOSYS && errno != EOPNOTSUPP)) { ret = -errno; virReportSystemError(errno, _("cannot allocate %llu bytes in file '%s'"), @@ -433,22 +433,19 @@ createRawFile(int fd, virStorageVolDefPtr vol, } #endif -remain = vol->target.allocation; +remain = vol->target.capacity; if (inputvol) { /* allow zero blocks to be skipped if we've requested sparse - * allocation (allocation < capacity) or we have already - * been able to allocate the required space. */ -bool want_sparse = !need_alloc || -(vol->target.allocation < inputvol->target.capacity); - + * allocation (allocation < capacity) + */ ret = virStorageBackendCopyToFD(vol, inputvol, fd, &remain, want_sparse, reflink_copy); if (ret < 0) goto cleanup; } -if (remain && need_alloc) { +if (remain && !want_sparse) { if (safezero(fd, vol->target.allocation - remain, remain) < 0) { ret = -errno; virReportSystemError(errno, _("cannot fill file '%s'"), diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 41de8df..b8ed006 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1976,11 +1976,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, if (newvol->target.capacity < origvol->target.capacity) newvol->target.capacity = origvol->target.capacity; -/* Make sure allocation is at least as large as the destination cap, - * to make absolutely sure we copy all possible contents */ -if (newvol->target.allocation < origvol->target.capacity) -newvol->target.allocation = origvol->target.capacity; - if (!backend->buildVolFrom) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("storage pool does not support" -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/2] Storage: Introduce shadow vol for refresh while the main vol builds.
Libvirt periodically refreshes all volumes in a storage pool, including the volumes being cloned. While cloning a storage volume from parent, we drop pool locks. Subsequent volume refresh sometimes changes allocation for an ongoing copy, and leads to corrupt images. Fix: Introduce a shadow volume that isolates the volume object under refresh from the base which has a copy ongoing. Signed-off-by: Prerna Saxena --- src/storage/storage_driver.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 57060ab..41de8df 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1898,7 +1898,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, { virStoragePoolObjPtr pool, origpool = NULL; virStorageBackendPtr backend; -virStorageVolDefPtr origvol = NULL, newvol = NULL; +virStorageVolDefPtr origvol = NULL, newvol = NULL, shadowvol = NULL; virStorageVolPtr ret = NULL, volobj = NULL; unsigned long long allocation; int buildret; @@ -2010,6 +2010,15 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, if (backend->createVol(obj->conn, pool, newvol) < 0) goto cleanup; +/* Make a shallow copy of the 'defined' volume definition, since the + * original allocation value will change as the user polls 'info', + * but we only need the initial requested values + */ +if (VIR_ALLOC(shadowvol) < 0) +goto cleanup; + +memcpy(shadowvol, newvol, sizeof(*newvol)); + pool->volumes.objs[pool->volumes.count++] = newvol; volobj = virGetStorageVol(obj->conn, pool->def->name, newvol->name, newvol->key, NULL, NULL); @@ -2029,7 +2038,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, virStoragePoolObjUnlock(origpool); } -buildret = backend->buildVolFrom(obj->conn, pool, newvol, origvol, flags); +buildret = backend->buildVolFrom(obj->conn, pool, shadowvol, origvol, flags); storageDriverLock(); virStoragePoolObjLock(pool); @@ -2071,6 +2080,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, cleanup: virObjectUnref(volobj); virStorageVolDefFree(newvol); +VIR_FREE(shadowvol); if (pool) virStoragePoolObjUnlock(pool); if (origpool) -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] [V3] Libvirt : Storage fixes
Here is V3 of storage fixes, which addresses review comments of v2. Summary: Prerna Saxena (2): Storage: Introduce shadow vol for refresh while the main vol builds. Storage : Fix cloning of raw, sparse volumes src/storage/storage_backend.c | 23 ++- src/storage/storage_driver.c | 19 --- 2 files changed, 22 insertions(+), 20 deletions(-) V2 : http://www.redhat.com/archives/libvir-list/2015-June/msg01052.html Changelog: Modified patches 1,2 per review comments. -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Storage : Fix cloning of raw, sparse volumes
On Tuesday 23 June 2015 06:29 PM, Ján Tomko wrote: > On Mon, Jun 22, 2015 at 05:09:18PM +0530, Prerna Saxena wrote: >> From: Prerna Saxena >> Date: Mon, 22 Jun 2015 02:54:32 -0500 >> >> When virsh vol-clone is attempted on a raw file where capacity > allocation, >> the resulting cloned volume has a size that matches the virtual-size of >> the parent; in place of matching its actual, disk size. >> This patch fixes the cloned disk to have same _allocated_size_ as >> the parent file from which it was cloned. >> >> Ref: http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html >> >> Also fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1130739 >> >> Signed-off-by: Prerna Saxena >> --- >> src/storage/storage_backend.c | 10 +- >> src/storage/storage_driver.c | 5 - >> 2 files changed, 5 insertions(+), 10 deletions(-) >> >> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c >> index ce59f63..c99b718 100644 >> --- a/src/storage/storage_backend.c >> +++ b/src/storage/storage_backend.c >> @@ -342,7 +342,7 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn >> ATTRIBUTE_UNUSED, >> goto cleanup; >> } >> >> -remain = vol->target.allocation; >> +remain = vol->target.capacity; >> >> if (inputvol) { >> int res = virStorageBackendCopyToFD(vol, inputvol, >> @@ -397,7 +397,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, >>virStorageVolDefPtr inputvol, >>bool reflink_copy) >> { >> -bool need_alloc = true; >> +bool need_alloc = !(inputvol && (inputvol->target.capacity > >> inputvol->target.allocation)); > Comparing 'inputvol->target.capacity > vol->target.allocation' would > allow creating a sparse volume from a non-sparse one and vice-versa > by specifying the correct allocation. Ok, I was not sure if libvirt wanted to do that. If the parent volume is a sparse volume, I'd expect the cloned volume to be sparse too. Likewise, for a non-sparse parent, the cloned volume should also be non-sparse. Should that not be something we honour in libvirt, when we clone ? >> int ret = 0; >> unsigned long long remain; >> >> @@ -420,7 +420,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, >> * to writing zeroes block by block in case fallocate isn't >> * available, and since we're going to copy data from another >> * file it doesn't make sense to write the file twice. */ >> -if (vol->target.allocation) { >> +if (vol->target.allocation && need_alloc) { >> if (fallocate(fd, 0, 0, vol->target.allocation) == 0) { >> need_alloc = false; >> } else if (errno != ENOSYS && errno != EOPNOTSUPP) { >> @@ -433,14 +433,14 @@ createRawFile(int fd, virStorageVolDefPtr vol, >> } >> #endif >> >> -remain = vol->target.allocation; >> +remain = vol->target.capacity; >> >> if (inputvol) { >> /* allow zero blocks to be skipped if we've requested sparse >> * allocation (allocation < capacity) or we have already >> * been able to allocate the required space. */ >> bool want_sparse = !need_alloc || >> -(vol->target.allocation < inputvol->target.capacity); >> +(inputvol->target.allocation < inputvol->target.capacity); > If allocation < capacity, then need_alloc is already false. > I was trying to accomodate the original usecase of need_alloc, but you are right. This will go away in the next version of this series, which I will post shortly. -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Introduce shadow vol for refresh while the main vol builds.
Hi Jan, Thanks for the review comments. On Tuesday 23 June 2015 06:21 PM, Ján Tomko wrote: > On Mon, Jun 22, 2015 at 05:07:26PM +0530, Prerna Saxena wrote: >> From: Prerna Saxena >> Date: Thu, 18 Jun 2015 05:05:09 -0500 >> >> Libvirt periodically refreshes all volumes in a storage pool, including >> the volumes being cloned. >> While cloning a storage volume from parent, we drop pool locks. Subsequent >> volume refresh sometimes changes allocation for an ongoing copy, and leads >> to corrupt images. >> Fix: Introduce a shadow volume that isolates the volume object under refresh >> from the base which has a copy ongoing. >> >> Signed-off-by: Prerna Saxena >> --- >> src/storage/storage_driver.c | 15 +-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c >> index 57060ab..4980546 100644 >> --- a/src/storage/storage_driver.c >> +++ b/src/storage/storage_driver.c >> @@ -1898,7 +1898,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, >> { >> virStoragePoolObjPtr pool, origpool = NULL; >> virStorageBackendPtr backend; >> -virStorageVolDefPtr origvol = NULL, newvol = NULL; >> +virStorageVolDefPtr origvol = NULL, newvol = NULL, shadowvol = NULL; >> virStorageVolPtr ret = NULL, volobj = NULL; >> unsigned long long allocation; >> int buildret; >> @@ -2010,6 +2010,17 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, >> if (backend->createVol(obj->conn, pool, newvol) < 0) >> goto cleanup; >> >> +/* Make a shallow copy of the 'defined' volume definition, since the >> + * original allocation value will change as the user polls 'info', >> + * but we only need the initial requested values >> + */ >> +if (VIR_ALLOC(shadowvol) < 0) { >> +newvol = NULL; > newvol has not been added to pool->volumes.objs yet, so we should free > it on the cleanup path. shadowvol should also be VIR_FREE'd. Thanks, I'd missed this. Will be addressed in subsequent patch. >> +goto cleanup; >> +} >> + >> +memcpy(shadowvol, newvol, sizeof(*newvol)); >> + >> pool->volumes.objs[pool->volumes.count++] = newvol; >> volobj = virGetStorageVol(obj->conn, pool->def->name, newvol->name, >>newvol->key, NULL, NULL); >> @@ -2029,7 +2040,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, >> virStoragePoolObjUnlock(origpool); >> } >> >> -buildret = backend->buildVolFrom(obj->conn, pool, newvol, origvol, >> flags); >> +buildret = backend->buildVolFrom(obj->conn, pool, shadowvol, origvol, >> flags); >> > A few lines below, there is one more usage of newvol after the pool has > been unlocked: newvol->target.allocation > > If the parallel volume refresh happened when the volume was not fully > allocated, this might not contain the intended allocation. > > Using shadowvol->target.allocation will behave the same regardless of a > parallel refresh (even though the buildVolFrom function might not honor > the allocation exactly). > > Right. The buildVolFrom() call completes an fsync and then returns. In my experimental runs, a parallel refresh would always happen by the time I got to this point; so this had missed me. But ofcourse we can never say that for sure. Will fix in the next patch. -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Storage : Fix cloning of raw, sparse volumes
From: Prerna Saxena Date: Mon, 22 Jun 2015 02:54:32 -0500 When virsh vol-clone is attempted on a raw file where capacity > allocation, the resulting cloned volume has a size that matches the virtual-size of the parent; in place of matching its actual, disk size. This patch fixes the cloned disk to have same _allocated_size_ as the parent file from which it was cloned. Ref: http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html Also fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1130739 Signed-off-by: Prerna Saxena --- src/storage/storage_backend.c | 10 +- src/storage/storage_driver.c | 5 - 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index ce59f63..c99b718 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -342,7 +342,7 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } -remain = vol->target.allocation; +remain = vol->target.capacity; if (inputvol) { int res = virStorageBackendCopyToFD(vol, inputvol, @@ -397,7 +397,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, bool reflink_copy) { -bool need_alloc = true; +bool need_alloc = !(inputvol && (inputvol->target.capacity > inputvol->target.allocation)); int ret = 0; unsigned long long remain; @@ -420,7 +420,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, * to writing zeroes block by block in case fallocate isn't * available, and since we're going to copy data from another * file it doesn't make sense to write the file twice. */ -if (vol->target.allocation) { +if (vol->target.allocation && need_alloc) { if (fallocate(fd, 0, 0, vol->target.allocation) == 0) { need_alloc = false; } else if (errno != ENOSYS && errno != EOPNOTSUPP) { @@ -433,14 +433,14 @@ createRawFile(int fd, virStorageVolDefPtr vol, } #endif -remain = vol->target.allocation; +remain = vol->target.capacity; if (inputvol) { /* allow zero blocks to be skipped if we've requested sparse * allocation (allocation < capacity) or we have already * been able to allocate the required space. */ bool want_sparse = !need_alloc || -(vol->target.allocation < inputvol->target.capacity); +(inputvol->target.allocation < inputvol->target.capacity); ret = virStorageBackendCopyToFD(vol, inputvol, fd, &remain, want_sparse, reflink_copy); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 4980546..c1dfe89 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1976,11 +1976,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, if (newvol->target.capacity < origvol->target.capacity) newvol->target.capacity = origvol->target.capacity; -/* Make sure allocation is at least as large as the destination cap, - * to make absolutely sure we copy all possible contents */ -if (newvol->target.allocation < origvol->target.capacity) -newvol->target.allocation = origvol->target.capacity; - if (!backend->buildVolFrom) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("storage pool does not support" -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Storage: Introduce shadow vol for refresh while the main vol builds.
From: Prerna Saxena Date: Thu, 18 Jun 2015 05:05:09 -0500 Libvirt periodically refreshes all volumes in a storage pool, including the volumes being cloned. While cloning a storage volume from parent, we drop pool locks. Subsequent volume refresh sometimes changes allocation for an ongoing copy, and leads to corrupt images. Fix: Introduce a shadow volume that isolates the volume object under refresh from the base which has a copy ongoing. Signed-off-by: Prerna Saxena --- src/storage/storage_driver.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 57060ab..4980546 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1898,7 +1898,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, { virStoragePoolObjPtr pool, origpool = NULL; virStorageBackendPtr backend; -virStorageVolDefPtr origvol = NULL, newvol = NULL; +virStorageVolDefPtr origvol = NULL, newvol = NULL, shadowvol = NULL; virStorageVolPtr ret = NULL, volobj = NULL; unsigned long long allocation; int buildret; @@ -2010,6 +2010,17 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, if (backend->createVol(obj->conn, pool, newvol) < 0) goto cleanup; +/* Make a shallow copy of the 'defined' volume definition, since the + * original allocation value will change as the user polls 'info', + * but we only need the initial requested values + */ +if (VIR_ALLOC(shadowvol) < 0) { +newvol = NULL; +goto cleanup; +} + +memcpy(shadowvol, newvol, sizeof(*newvol)); + pool->volumes.objs[pool->volumes.count++] = newvol; volobj = virGetStorageVol(obj->conn, pool->def->name, newvol->name, newvol->key, NULL, NULL); @@ -2029,7 +2040,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, virStoragePoolObjUnlock(origpool); } -buildret = backend->buildVolFrom(obj->conn, pool, newvol, origvol, flags); +buildret = backend->buildVolFrom(obj->conn, pool, shadowvol, origvol, flags); storageDriverLock(); virStoragePoolObjLock(pool); -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/2] Storage fixes for libvirt.
From: Prerna Saxena Date: Mon, 22 Jun 2015 06:12:24 -0500 This is the second version of storage fixes. V1 : http://www.redhat.com/archives/libvir-list/2015-May/msg00050.html Summary: Prerna Saxena (2): Storage: Introduce shadow vol for refresh while the main vol builds. Storage : Fix cloning of raw, sparse volumes src/storage/storage_backend.c | 10 +- src/storage/storage_driver.c | 20 +--- 2 files changed, 18 insertions(+), 12 deletions(-) Changelog: -- 1. Dropped patch 1/2 of v1 as per jtomko's suggestion; and introduced a new patch for shadow-volume based cloning. 2. Reworked patch 2/2 incorporating review comments. -- Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
On Tuesday 05 May 2015 04:22 PM, Ján Tomko wrote: > On Tue, May 05, 2015 at 03:24:31PM +0530, Prerna Saxena wrote: >> On Tuesday 05 May 2015 03:20 PM, Prerna Saxena wrote: >>> On Tuesday 05 May 2015 01:52 PM, Ján Tomko wrote: >>>> On Tue, May 05, 2015 at 08:43:21AM +0530, Prerna Saxena wrote: >>>>> Libvirt periodically calls 'stat' on all volumes in a storage pool, >>>>> to update fields such as 'target.allocation'. >>>>> >>>>> The operation doesnt make sense for a volume which is curently being >>>>> allocated. >>>> From the comments in the storage driver, the point of allowing refresh >>>> for a volume that is currently being allocated is to track the progress >>>> of the allocation. >>>> >>>>> Also, the 'target.allocation' sub-field is taken into account while >>>>> copying a raw image. >>>>> To suppress any (potential) corruption, libvirt must not attempt to >>>>> refresh a volume currently being built. >>>> What would be the corruption? >>>> >>>> We do not allow using a volume that is currently building as a >>>> source for cloning the volume - storageVolCreateXMLFrom checks for >>>> origvol->building: >>>> >>>> if (origvol->building) { >>>> virReportError(VIR_ERR_OPERATION_INVALID, >>>>_("volume '%s' is still being allocated."), >>>>origvol->name); >>>> goto cleanup; >>>> } >>>> >>> While running libvirt on PowerPC, I saw an interesting scenario. The >>> 'target.allocation' field seemed to change for a volume getting allocated, >>> and this would lead to incomplete copy. This would >>> happen at random intervals, not deterministically. While looking through >>> the code, I found this to be the other place in code where the same field >>> seemed to change without a lock. Hence the patch. >>> > Oh, I was thinking of the soure volume for some reason. > > We correctly lock the pool before calling refreshVol, so changing the > object should not be an issue. > I think the bug is in storageVolCreateXMLFrom - it drops all the locks, > but expects the allocation not to change. Yes, and so I sent this patch that blocks a refresh for volumes being created. > In storageVolCreateXML we work around this by creating a shallow copy of > the volume. > >>> I have sent the second patch which fixes the erring code too : >>> >>> -remain = vol->target.allocation; >>> +remain = inputvol->target.capacity; >>> >> More fundamental question -- why do we offload the copying of non-raw images >> to qemu-img tool, but make libvirt responsible for copying raw volumes ? >> Would it not be better if libvirt called on 'qemu-img' to copy all types of >> volumes, including raw ones ? >> > This way, libvirt can create raw volumes even without qemu-img > installed. I don't know if there's any other reason. > I'm sorry, libvirt does not copy raw volumes as a 'fallback mechanism'. Libvirt chooses to copy raw volumes on its own, but calls on qemu-img to copy all other volume types. Is it not better to let qemu-img copy all volume types , including raw ones ? I wanted to check if there are reasons for this decision ? I'll be happy if the community could throw some light. Also cc'ing Cole, who had put in some fixes in this area. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
On Tuesday 05 May 2015 03:20 PM, Prerna Saxena wrote: > On Tuesday 05 May 2015 01:52 PM, Ján Tomko wrote: >> On Tue, May 05, 2015 at 08:43:21AM +0530, Prerna Saxena wrote: >>> Libvirt periodically calls 'stat' on all volumes in a storage pool, >>> to update fields such as 'target.allocation'. >>> >>> The operation doesnt make sense for a volume which is curently being >>> allocated. >> From the comments in the storage driver, the point of allowing refresh >> for a volume that is currently being allocated is to track the progress >> of the allocation. >> >>> Also, the 'target.allocation' sub-field is taken into account while copying >>> a raw image. >>> To suppress any (potential) corruption, libvirt must not attempt to refresh >>> a volume currently being built. >> What would be the corruption? >> >> We do not allow using a volume that is currently building as a >> source for cloning the volume - storageVolCreateXMLFrom checks for >> origvol->building: >> >> if (origvol->building) { >> virReportError(VIR_ERR_OPERATION_INVALID, >>_("volume '%s' is still being allocated."), >>origvol->name); >> goto cleanup; >> } >> > While running libvirt on PowerPC, I saw an interesting scenario. The > 'target.allocation' field seemed to change for a volume getting allocated, > and this would lead to incomplete copy. This would > happen at random intervals, not deterministically. While looking through the > code, I found this to be the other place in code where the same field seemed > to change without a lock. Hence the patch. > > I have sent the second patch which fixes the erring code too : > > -remain = vol->target.allocation; > +remain = inputvol->target.capacity; > More fundamental question -- why do we offload the copying of non-raw images to qemu-img tool, but make libvirt responsible for copying raw volumes ? Would it not be better if libvirt called on 'qemu-img' to copy all types of volumes, including raw ones ? -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
On Tuesday 05 May 2015 01:52 PM, Ján Tomko wrote: > On Tue, May 05, 2015 at 08:43:21AM +0530, Prerna Saxena wrote: >> Libvirt periodically calls 'stat' on all volumes in a storage pool, >> to update fields such as 'target.allocation'. >> >> The operation doesnt make sense for a volume which is curently being >> allocated. > From the comments in the storage driver, the point of allowing refresh > for a volume that is currently being allocated is to track the progress > of the allocation. > >> Also, the 'target.allocation' sub-field is taken into account while copying >> a raw image. >> To suppress any (potential) corruption, libvirt must not attempt to refresh >> a volume currently being built. > What would be the corruption? > > We do not allow using a volume that is currently building as a > source for cloning the volume - storageVolCreateXMLFrom checks for > origvol->building: > > if (origvol->building) { > virReportError(VIR_ERR_OPERATION_INVALID, >_("volume '%s' is still being allocated."), >origvol->name); > goto cleanup; > } > While running libvirt on PowerPC, I saw an interesting scenario. The 'target.allocation' field seemed to change for a volume getting allocated, and this would lead to incomplete copy. This would happen at random intervals, not deterministically. While looking through the code, I found this to be the other place in code where the same field seemed to change without a lock. Hence the patch. I have sent the second patch which fixes the erring code too : -remain = vol->target.allocation; +remain = inputvol->target.capacity; Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Storage : Fix cloning of raw, sparse volumes.
When virsh vol-clone is attempted on a raw file where capacity > allocation, the resulting cloned volume has a size that matches the virtual-size of the parent; in place of matching its actual, disk size. This patch fixes the cloned disk to have same _allocated_size_ as the parent file from which it was cloned. Reference: https://www.redhat.com/archives/libvir-list/2014-September/msg00064.html Also fixes : https://bugzilla.redhat.com/show_bug.cgi?id=1130739 Signed-off-by: Prerna Saxena --- src/storage/storage_backend.c | 2 +- src/storage/storage_driver.c | 5 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 355fc7f..1a7c0cc 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -429,7 +429,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, } #endif -remain = vol->target.allocation; +remain = inputvol->target.capacity; if (inputvol) { /* allow zero blocks to be skipped if we've requested sparse diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index ac4a74a..c511035 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1990,11 +1990,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, if (newvol->target.capacity < origvol->target.capacity) newvol->target.capacity = origvol->target.capacity; -/* Make sure allocation is at least as large as the destination cap, - * to make absolutely sure we copy all possible contents */ -if (newvol->target.allocation < origvol->target.capacity) -newvol->target.allocation = origvol->target.capacity; - if (!backend->buildVolFrom) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("storage pool does not support" -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Storage: Suppress metadata refresh for volumes being built.
Libvirt periodically calls 'stat' on all volumes in a storage pool, to update fields such as 'target.allocation'. The operation doesnt make sense for a volume which is curently being allocated. Also, the 'target.allocation' sub-field is taken into account while copying a raw image. To suppress any (potential) corruption, libvirt must not attempt to refresh a volume currently being built. Signed-off-by: Prerna Saxena --- src/storage/storage_backend.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 289f454..355fc7f 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1576,6 +1576,9 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, { int ret; +if (vol->building) +return 0; + if ((ret = virStorageBackendUpdateVolTargetInfo(&vol->target, withBlockVolFormat, openflags)) < 0) -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Storage : Fixes for cloning raw volumes
From: Prerna Saxena Date: Mon, 4 May 2015 12:00:46 -0700 This series has some long-overdue fixes for copying of raw storage volumes with libvirt. Prerna Saxena (2): Storage : Suppress metadata refresh for volumes being built. Storage : Fix cloning of raw, sparse volumes. src/storage/storage_backend.c | 5 - src/storage/storage_driver.c | 5 - 2 files changed, 4 insertions(+), 6 deletions(-) -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] schema: Allow multiple machines for sparc VMs
On Monday 13 April 2015 07:50 PM, Daniel P. Berrange wrote: > On Mon, Apr 13, 2015 at 04:14:53PM +0200, Martin Kletzander wrote: >> Use the same pattern as there is for x86 machines. >> >> Signed-off-by: Martin Kletzander >> --- >> docs/schemas/domaincommon.rng | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng >> index 03fd541..80b30df 100644 >> --- a/docs/schemas/domaincommon.rng >> +++ b/docs/schemas/domaincommon.rng >> @@ -384,7 +384,9 @@ >> >> >> >> - sun4m >> + >> +[a-zA-Z0-9_\.\-]+ >> + >> >> >> > I think you could probably simplify this all much more. All these > architecture specific blocks of machine type names should just be > deleted and so this: > > > > > > > > > > > > > > > > hvm > > > > Would simplify to just > > > > > > > i686 > others... > > > > > > > [a-zA-Z0-9_\.\-]+ > > > > > Hi Martin, Daniel, I have not been able to try this patch, it fails with this error : error: internal error: Unable to parse RNG /test-libvirt/share/libvirt/schemas/domain.rng: Reference osexe has no matching definition Internal found no define for ref osexe However, had some concerns purely by looking at this patch. This change is very x86-centric, it does not respect other architectures. I think the rationale for simplifying domaincommon.rng would have been to group all types that obey this pattern string: [a-zA-Z0-9_\.\-]+ However, this regex does not conform to machine types for _all_ architectures. As an example, see this : s390 s390x s390 s390-virtio s390-ccw s390-ccw-virtio The s390 arch only allows four machine names : "s390", "s390-virtio", "s390-ccw", "s390-ccw-virtio". With the patch you suggest, even a string such as "abcdefg" will become a legitimate machine type for s390x, which seems like an odd thing. Likewise, ppc64[le] architecture allows only strings such as pseries, pseries-2.1, pseries-2.2 .. This patch will allow any random machine name, which seems somewhat odd to me. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] PowerPC: Replace hardcoded values of 'pseries-2*' machine in schema
From: Prerna Saxena Date: Mon, 6 Apr 2015 22:56:25 -0500 Qemu-system-ppc64 has introduced new variants of pseries machine type, such as 'pseries', 'pseries-2.1'..'pseries-2.3'. Replace the hardcoded values in the schema with a regex that captures this. Signed-off-by: Prerna Saxena --- docs/schemas/domaincommon.rng | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7406d92..a192637 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -421,9 +421,9 @@ -pseries -pseries-2.1 -pseries-2.2 + + (pseries)(\-2\.[1-9])? + -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 00/10] Add support for memory hotplug
Hi Peter, While playing around with memory hotplug implementation, I found that the guest XML isnt updated after a successful hotplug operation : [root@kop2 test-libvirt]# ./bin/virsh attach-device rhel71-be /home/prerna/mem-hp.xml Device attached successfully [root@kop2 test-libvirt]# ./bin/virsh qemu-monitor-command rhel71-be --hmp "info memdev" memory backend: 0 size: 1073741824 merge: true dump: true prealloc: false policy: default host nodes: [root@kop2 test-libvirt]# ./bin/virsh edit rhel71-be < Does not reflect an updated memory value or a separate dimm device > I have observed this on PowerKVM. Wondering what is a good place to observe the hot-added memory : Should this reflect as an updated value for current memory in domain XML, or should this show up as a memory device of type: 4096 1-3 524287 1 I would be happy to work on a patch to fix this. Pls provide me your thoughts. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] PowerPC : Do not allow an empty model spec for 'host-model'
On Friday 20 March 2015 01:51 PM, Ján Tomko wrote: > On Mon, Mar 16, 2015 at 04:56:49PM +0530, Prerna Saxena wrote: >> [PATCH] PowerPC : Do not allow an empty model spec for 'host-model' >> >> On PowerPC, a guest VM having CPU mode as 'host-model' >> represents a 'compat' mode VM. This cannot have a NULL >> CPU model. > I thought the compat= mode was only used when mode == HOST_MODEL > and a model is specified. And HOST_MODEL with no model behaves like on > x86_64 - copies the features from the host capabilities. > > Was this functionality broken by commit addce06 or did it never produce > useful results? > > Jan > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list Hi Jan, This commit does not break anything. It addresses a few corner cases not completely addressed by commits addce06 & 5e4f49ab8aa2. PowerPC pseries KVM is a paravirtualized platform, wherein there are only 2 allowed vcpu configurations of running a guest : 1) Native mode, where the guest sees the same vcpu model as the host -- this is reflected in libvirt by "host-passthrough" mode; 2) Compat mode, where the physical processor itself runs in binary compatibility with an older cpu model. This is marked in libvirt by "host-model" mode, which takes on an additional argument -- the guest CPU model which needs to be run. This was introduced by commit addce06. PowerKVM, being a paravirt platform, does not emulate a guest vcpu based on features copied from host. This behaviour is unlike x86 KVM. Hence , the host-model mode on PowerKVM needs to error out in case the model which needs to be run in binary compatibility is not specified by user. The reason for this commit was a bug seen even after 5e4f49ab8aa2. A null cpu model XML was causing an incorrect "best-fit" model (just like x86) to be passed to the VM after a save/restore. Hence the need for this check. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] PowerPC : Do not allow an empty model spec for 'host-model'
[PATCH] PowerPC : Do not allow an empty model spec for 'host-model' On PowerPC, a guest VM having CPU mode as 'host-model' represents a 'compat' mode VM. This cannot have a NULL CPU model. This commit forbids such a guest definition. Signed-off-by: Prerna Saxena --- src/cpu/cpu_powerpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index c77374c..62ad92b 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -549,12 +549,12 @@ ppcUpdate(virCPUDefPtr guest, const virCPUDef *host) { switch ((virCPUMode) guest->mode) { -case VIR_CPU_MODE_HOST_MODEL: case VIR_CPU_MODE_HOST_PASSTHROUGH: guest->match = VIR_CPU_MATCH_EXACT; virCPUDefFreeModel(guest); return virCPUDefCopyModel(guest, host, true); +case VIR_CPU_MODE_HOST_MODEL: case VIR_CPU_MODE_CUSTOM: return 0; -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Tests : Add test for 'ppc64le' architecture.
On Wednesday 04 March 2015 09:28 PM, Ján Tomko wrote: > On Thu, Feb 26, 2015 at 10:45:54PM +0530, Prerna Saxena wrote: >> Tests : Add test for 'ppc64le' architecture. >> >> Signed-off-by: Prerna Saxena >> --- >> .../qemuxml2argv-pseries-cpu-le.args | 7 + >> .../qemuxml2argv-pseries-cpu-le.xml| 17 >> tests/qemuxml2argvtest.c | 2 ++ >> tests/testutilsqemu.c | 30 >> ++ >> 4 files changed, 56 insertions(+) >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml >> >> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args >> b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args >> new file mode 100644 >> index 000..f4f8d5e >> --- /dev/null >> +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args >> @@ -0,0 +1,7 @@ >> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ >> +QEMU_AUDIO_DRV=none /usr/bin/qemu-system-ppc64 -S -M pseries \ >> +-m 512 -smp 1 -nographic -nodefconfig -nodefaults \ >> +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ >> +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb \ >> +-chardev pty,id=charserial0 \ >> +-device spapr-vty,chardev=charserial0,reg=0x3000 > I don't see any reference to little endian on the command line. > > If the domain can only use the same endianness as the host system, do we > need to check if the guest arch matches the host arch somewhere? > > Yes, SLOF in qemu-system-ppc64 can correctly initialize KVM based on the endianness discovered for the guest, so the command line stays the same for both ppc64 & ppc64le guests. However, we need libvirt to recognize ppc64le as a valid arch, since this is used by management utilities that depend on libvirt -- a classic example being virt-install. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] PowerPC : Miscellaneous fixes for 'ppc64le' architecture.
On Tuesday 03 March 2015 03:29 PM, Michal Privoznik wrote: > On 26.02.2015 18:09, Prerna Saxena wrote: >> >From a28ef5a3e7b9cb023948cf97d9f472bb3a1e06d3 Mon Sep 17 00:00:00 2001 >> From: Prerna Saxena >> Date: Thu, 26 Feb 2015 22:31:05 +0530 >> >> This series adds few miscellaneous fixes for PowerPC 64-bit Little >> Endian Architecture. >> >> Changelog : >> == >> v1 of Patch 1/2 already posted and acked : >> https://www.redhat.com/archives/libvir-list/2015-February/msg00486.html >> Patch 2/2 adds a testcase to supplement patch 1. >> >> Prerna Saxena (2): >> PowerPC: Augment XML schema to include 'ppc64le' arch and newer >> pseries-2.* machine types. >> Tests : Add test for 'ppc64le' architecture. >> >> docs/schemas/basictypes.rng| 1 + >> docs/schemas/domaincommon.rng | 7 - >> .../qemuxml2argv-pseries-cpu-le.args | 7 + >> .../qemuxml2argv-pseries-cpu-le.xml| 17 >> tests/qemuxml2argvtest.c | 2 ++ >> tests/testutilsqemu.c | 30 >> ++ >> 6 files changed, 63 insertions(+), 1 deletion(-) >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml >> > I've tweaked the commit messages a bit, ACKed and pushed. > > Michal > Thank you :-) -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Fwd: [PATCH 0/2] PowerPC : Miscellaneous fixes for 'ppc64le' architecture.
Ping !` Can you pls let me know if this suffices ? Regards, Prerna Forwarded Message Subject:[libvirt] [PATCH 0/2] PowerPC : Miscellaneous fixes for 'ppc64le' architecture. Date: Thu, 26 Feb 2015 22:39:20 +0530 From: Prerna Saxena To: libvirt mailing list CC: Ján Tomko >From a28ef5a3e7b9cb023948cf97d9f472bb3a1e06d3 Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Thu, 26 Feb 2015 22:31:05 +0530 This series adds few miscellaneous fixes for PowerPC 64-bit Little Endian Architecture. Changelog : == v1 of Patch 1/2 already posted and acked : https://www.redhat.com/archives/libvir-list/2015-February/msg00486.html Patch 2/2 adds a testcase to supplement patch 1. Prerna Saxena (2): PowerPC: Augment XML schema to include 'ppc64le' arch and newer pseries-2.* machine types. Tests : Add test for 'ppc64le' architecture. docs/schemas/basictypes.rng| 1 + docs/schemas/domaincommon.rng | 7 - .../qemuxml2argv-pseries-cpu-le.args | 7 + .../qemuxml2argv-pseries-cpu-le.xml| 17 tests/qemuxml2argvtest.c | 2 ++ tests/testutilsqemu.c | 30 ++ 6 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] PowerPC : Miscellaneous fixes for 'ppc64le' architecture.
>From a28ef5a3e7b9cb023948cf97d9f472bb3a1e06d3 Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Thu, 26 Feb 2015 22:31:05 +0530 This series adds few miscellaneous fixes for PowerPC 64-bit Little Endian Architecture. Changelog : == v1 of Patch 1/2 already posted and acked : https://www.redhat.com/archives/libvir-list/2015-February/msg00486.html Patch 2/2 adds a testcase to supplement patch 1. Prerna Saxena (2): PowerPC: Augment XML schema to include 'ppc64le' arch and newer pseries-2.* machine types. Tests : Add test for 'ppc64le' architecture. docs/schemas/basictypes.rng| 1 + docs/schemas/domaincommon.rng | 7 - .../qemuxml2argv-pseries-cpu-le.args | 7 + .../qemuxml2argv-pseries-cpu-le.xml| 17 tests/qemuxml2argvtest.c | 2 ++ tests/testutilsqemu.c | 30 ++ 6 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Tests : Add test for 'ppc64le' architecture.
Tests : Add test for 'ppc64le' architecture. Signed-off-by: Prerna Saxena --- .../qemuxml2argv-pseries-cpu-le.args | 7 + .../qemuxml2argv-pseries-cpu-le.xml| 17 tests/qemuxml2argvtest.c | 2 ++ tests/testutilsqemu.c | 30 ++ 4 files changed, 56 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args new file mode 100644 index 000..f4f8d5e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +QEMU_AUDIO_DRV=none /usr/bin/qemu-system-ppc64 -S -M pseries \ +-m 512 -smp 1 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb \ +-chardev pty,id=charserial0 \ +-device spapr-vty,chardev=charserial0,reg=0x3000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml new file mode 100644 index 000..b791a73 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-le.xml @@ -0,0 +1,17 @@ + + QEMUGuest1 + 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 + 524288 + 1 + +hvm + + + +/usr/bin/qemu-system-ppc64 + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39ed66b..982019e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1341,6 +1341,8 @@ mymain(void) QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-cpu-compat", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +DO_TEST("pseries-cpu-le", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST, +QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 7b26e50..1fd7c96 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -86,6 +86,33 @@ static int testQemuAddPPC64Guest(virCapsPtr caps) return -1; } +static int testQemuAddPPC64LEGuest(virCapsPtr caps) +{ +static const char *machine[] = { "pseries" }; +virCapsGuestMachinePtr *machines = NULL; +virCapsGuestPtr guest; + +machines = virCapabilitiesAllocMachines(machine, 1); +if (!machines) +goto error; + +guest = virCapabilitiesAddGuest(caps, "hvm", VIR_ARCH_PPC64LE, +"/usr/bin/qemu-system-ppc64", NULL, + 1, machines); +if (!guest) +goto error; + +if (!virCapabilitiesAddGuestDomain(guest, "qemu", NULL, NULL, 0, NULL)) +goto error; + +return 0; + + error: +/* No way to free a guest? */ +virCapabilitiesFreeMachines(machines, 1); +return -1; +} + static int testQemuAddPPCGuest(virCapsPtr caps) { static const char *machine[] = { "g3beige", @@ -332,6 +359,9 @@ virCapsPtr testQemuCapsInit(void) if (testQemuAddPPC64Guest(caps)) goto cleanup; +if (testQemuAddPPC64LEGuest(caps)) +goto cleanup; + if (testQemuAddPPCGuest(caps)) goto cleanup; -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] PowerPC: Augment XML schema to include 'ppc64le' arch ; newer pseries-2.* machine types.
>From 7128e773058751e4d1024ef7d8e4ad286c93ba55 Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Thu, 26 Feb 2015 08:10:58 -0600 Subject: [PATCH 1/2] PowerPC: Augment XML schema to include 'ppc64le' arch and newer pseries-2.* machine types. Acked-by: Ján Tomko Signed-off-by: Prerna Saxena --- docs/schemas/basictypes.rng | 1 + docs/schemas/domaincommon.rng | 7 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 2bc9c1b..3e1841c 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -333,6 +333,7 @@ parisc64 ppc ppc64 + ppc64le ppcemb s390 s390x diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 27a24b4..8613125 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -412,13 +412,18 @@ - ppc64 + +ppc64 +ppc64le + pseries +pseries-2.1 +pseries-2.2 -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch 0/4] PowerPC fixes for libvirt
On Tuesday 17 February 2015 06:33 PM, Ján Tomko wrote: > On Sun, Feb 15, 2015 at 09:45:25AM +0530, Prerna Saxena wrote: >> This patch set addresses some miscellaneous fixes for libvirt on PowerPC. >> >> Details: >> Patch 1/4 : This adds 'qemu-system-ppc64' as the default emulator for ppc64 >> & ppc64le guests. >> Patch 2/4 : Fixes a small error where not specifying a CPU model leads to a >> NULL compat specification. > I have pushed the first two patches. > >> Patch 3/4 : This introduces 'ppc64le' and newer pseries machine types to >> domain schema. >> Patch 4/4 : Forbids floppy devices in domain XML. > These would be better if they included some test cases (so would 2/4, > but adding a different host cpu to the test suite is not that trivial). > > Jan Thanks for pushing patches 1,2. I will add testcases and resend patch #3. As discussed with you, I will rework Patch #4 in line with https://www.redhat.com/archives/libvir-list/2015-February/msg00410.html Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] PowerPC : Do not allow floppy disks in domain XML.
Subject: [PATCH 4/4] PowerPC : Do not allow floppy disks in domain XML. PowerKVM does not support floppy disks. Ensure that libvirt honours this. Fixes : https://bugzilla.redhat.com/show_bug.cgi?id=1180486 Signed-off-by: Prerna Saxena --- src/conf/domain_conf.c | 8 1 file changed, 8 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6a57d80..df3a768 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13892,6 +13892,14 @@ virDomainDefParseXML(xmlDocPtr xml, if (!disk) goto error; +if (ARCH_IS_PPC64(def->os.arch) && +STREQLEN(def->os.machine, "pseries", 7) && +disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Pseries machine does not support floppy device")); +goto error; +} + virDomainDiskInsertPreAlloced(def, disk); } VIR_FREE(nodes); -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] PowerPC: Augment XML schema to include 'ppc64le' arch and pseries* machine types.
PowerPC: Augment XML schema to include 'ppc64le' arch and newer pseries-2.* machine types. Signed-off-by: Prerna Saxena --- docs/schemas/basictypes.rng | 1 + docs/schemas/domaincommon.rng | 7 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 2bc9c1b..3e1841c 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -333,6 +333,7 @@ parisc64 ppc ppc64 + ppc64le ppcemb s390 s390x diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index d467dce..3a91365 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -412,13 +412,18 @@ - ppc64 + +ppc64 +ppc64le + pseries +pseries-2.1 +pseries-2.2 -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] PowerPC : Forbid NULL CPU model with 'host-model' mode.
PowerPC : Forbid NULL CPU model with 'host-model' mode in qemu command line. This ensures that an XML such as following: ... ... will not generate a '-cpu host,compat=(null)' command line with qemu-system-ppc64. Signed-off-by: Prerna Saxena --- src/qemu/qemu_command.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9c25788..317bb19 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6685,7 +6685,8 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, virBufferAddLit(buf, "host"); if (ARCH_IS_PPC64(def->os.arch) && -cpu->mode == VIR_CPU_MODE_HOST_MODEL) { +cpu->mode == VIR_CPU_MODE_HOST_MODEL && +def->cpu->model != NULL) { virBufferAsprintf(buf, ",compat=%s", def->cpu->model); } else { featCpu = cpu; -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] PowerPC : Make 'qemu-system-ppc64' the default emulator on ppc64[le].
PowerPC : Explicitly associate 'qemu-system-ppc64' as the default emulator for all 64-bit PowerPC guests ( both Big & Little Endian ) Signed-off-by: Prerna Saxena --- src/qemu/qemu_capabilities.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a04095e..4596f6c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -673,9 +673,14 @@ virQEMUCapsFindBinaryForArch(virArch hostarch, virArch guestarch) { char *ret; -const char *archstr = virQEMUCapsArchToString(guestarch); +const char *archstr; char *binary; +if (ARCH_IS_PPC64(guestarch)) +archstr = virQEMUCapsArchToString(VIR_ARCH_PPC64); +else +archstr = virQEMUCapsArchToString(guestarch); + if (virAsprintf(&binary, "qemu-system-%s", archstr) < 0) return NULL; -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [Patch 0/4] PowerPC fixes for libvirt
This patch set addresses some miscellaneous fixes for libvirt on PowerPC. Details: Patch 1/4 : This adds 'qemu-system-ppc64' as the default emulator for ppc64 & ppc64le guests. Patch 2/4 : Fixes a small error where not specifying a CPU model leads to a NULL compat specification. Patch 3/4 : This introduces 'ppc64le' and newer pseries machine types to domain schema. Patch 4/4 : Forbids floppy devices in domain XML. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] [Patch] Make hugepage testcase arch-agnostic
Hi, I have attached this patch as a response to a recent failure observed on PowerPC architecture by commit 311b4a67. This patch introduces a check for dynamically obtaining system page size for test hugepages-pages6 under 'qemuxml2argv' suite. ( See patch for more verbose problem description) This patch is not the most perfect implementation -- it fails syntax check; and has a Makefile-driven cleanup pending. I will be happy to deck it up and send it if the community concurs with this approach. We could also implement this via a shell script ( just like 'virt-test-aa-helper') but I couldnt find an easy way to determine host page size. Awaiting community responses, Prerna >From 8a64d4d22e2e65158d3caa45b615ca9a263f841f Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 2 Feb 2015 10:48:48 +0530 Subject: [PATCH] Commit 311b4a67 introduces a test for normal-page backed guest XML. However, it hardcodes the page size to 4 KB which is only valid for Intel Make check consequently fails on PowerPC where page size is 64KB This makes the hugepages-pages6 test more modular, and enables the page size to be picked up at runtime. --- .../qemuxml2argv-hugepages-pages6.template | 32 ++ tests/qemuxml2argvtest.c | 24 +++- 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.template diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.template b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.template new file mode 100644 index 000..8b9b995 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.template @@ -0,0 +1,32 @@ + + SomeDummyHugepagesGuest + ef1bdff4-27f3-4e85-a807-5fb4d58463cc + 1048576 + 1048576 + + + + + + 2 + +hvm + + + + destroy + restart + destroy + +/usr/bin/qemu + + + + + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 89afa81..a16d937 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -479,6 +479,11 @@ mymain(void) { int ret = 0; bool skipLegacyCPUs = false; +const long system_page_size = sysconf(_SC_PAGESIZE) / 1024; +int fd_in; +FILE *f_out; +char *template, *xml = NULL; +char buf[1000]; abs_top_srcdir = getenv("abs_top_srcdir"); if (!abs_top_srcdir) @@ -702,7 +707,24 @@ mymain(void) DO_TEST_FAILURE("hugepages-pages4", QEMU_CAPS_MEM_PATH, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("hugepages-pages5", QEMU_CAPS_MEM_PATH); -DO_TEST("hugepages-pages6", NONE); + +if (virAsprintf(&template, "%s/qemuxml2argvdata/qemuxml2argv-%s.template", +abs_srcdir, "hugepages-pages6") < 0 || +virAsprintf(&xml, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", +abs_srcdir, "hugepages-pages6") < 0) +return EXIT_FAILURE; +fd_in = open(template, O_RDONLY); +f_out = fopen(xml, "w"); + +if ( fd_in != -1 && f_out != NULL ) { +if(read(fd_in, &buf, sizeof(buf))) { +fprintf(f_out, buf,system_page_size); +fclose(f_out); +close(fd_in); +DO_TEST("hugepages-pages6", NONE); +} +} + DO_TEST("nosharepages", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MEM_MERGE); DO_TEST("disk-cdrom", NONE); DO_TEST("disk-cdrom-network-http", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE, -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cpu: add Freescale ppc64 CPU models
On Tuesday 27 January 2015 06:53 AM, hong-hua@freescale.com wrote: > Hi Prerna, > > Currently, cpu_powerpc.c only support IBM ppc64 CPU models. > Could your please help review the previous patch? With this patch, Freescale > ppc64 CPU modesl could also be recognized. > > # virsh cpu-models ppc64 > POWER7 > POWER7_v2.1 > POWER7_v2.3 > POWER7+_v2.1 > POWER8_v1.0 > POWERPC_e5500_v10 > POWERPC_e5500_v11 > POWERPC_e5500_v12 > POWERPC_e5500_v1020 > POWERPC_e6500_v10 > POWERPC_e6500_v20 > POWERPC_e6500_v120 > > But PowerPC 32bit is still recognized as 'generic architecture'. > # virsh cpu-models ppc > error: failed to get CPU model names > error: internal error: cannot find CPU map for generic architecture > > Is there any plan to support ppc 32bit CPU models in cpu_powerpc.c file? > Hi Olivia, The current implementation only supports ppc64 architecture. Either the current driver needs to be extended for ppc 32 bit too, or new handler functions need to be implemented for 32 bit PowerPC. I do not have a 32 bit PowerPC system to try this. Patches are welcome, and I will be happy to help with reviews :-) Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cpu: add Freescale ppc64 CPU models
On Friday 23 January 2015 08:42 AM, Olivia Yin wrote: > Signed-off-by: Olivia Yin > --- > src/cpu/cpu_map.xml | 38 +- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml > index bd9b056..c34874e 100644 > --- a/src/cpu/cpu_map.xml > +++ b/src/cpu/cpu_map.xml > @@ -1,4 +1,4 @@ > - > +n This looks to be a typo. > > > > @@ -600,6 +600,7 @@ > > > > + > > > > @@ -657,5 +658,40 @@ > > > > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > > Rest of the patch looks good. However, I had an observation. It appears that POWERPC_e6500_v10 , .._v20 & _v120 seem variants of the same processor family -- they share the same higher 16 bits of PVR. Do you need to specifically expose the different variants of these models in the management stack ? Or the purpose of adding these entries is merely to enable libvirt to run on these boards ? If the latter describes your need, you just need to add a generic entry for this family, such as : + + + + Libvirt driver for ppc64 currently has support to fallback to a generic entry for a given model if the exact PVR isnt found. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [Build-breaker : 1.2.12] Suppress compilation without dbus headers
On Monday 26 January 2015 02:47 PM, Daniel P. Berrange wrote: > On Sun, Jan 25, 2015 at 05:56:25PM +0530, Prerna Saxena wrote: >> Hi, >> While testing 1.2.12 rc2 on Powerpc, Fedora 21, I hit a bunch of build >> failures in absence of dbus-devel : >> >> src/util/virsystemd.c:284:17: note: in expansion of macro 'STREQ_NULLABLE' >> if (STREQ_NULLABLE("org.freedesktop.DBus.Error.UnknownMethod", >> ^ >> >> src/util/virsystemd.c:288:17: error: implicit declaration of function >> 'dbus_error_free' [-Werror=implicit-function-declaration] >> dbus_error_free(&error); >> >> >> Found that this was because commit 318df5a05 needs Dbus libraries for >> compilation, and configure didnt mark "with_dbus" to be mandatory. In this >> case, the compilation itself failed in a much uglier >> fashion on my system where dbus-devel was absent. >> >> The following patch introduces a compile error when dbus-devel is absent, so >> that the build itself proceeds later without cryptic errors. >> >> This has also been reported on the list : >> https://www.redhat.com/archives/libvir-list/2015-January/msg00641.html > I've already posted a fix for this problem > > https://www.redhat.com/archives/libvir-list/2015-January/msg00869.html > > Oh ! I had probably missed this fix on the list. Thanks, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] [Build-breaker : 1.2.12] Suppress compilation without dbus headers
Hi, While testing 1.2.12 rc2 on Powerpc, Fedora 21, I hit a bunch of build failures in absence of dbus-devel : src/util/virsystemd.c:284:17: note: in expansion of macro 'STREQ_NULLABLE' if (STREQ_NULLABLE("org.freedesktop.DBus.Error.UnknownMethod", ^ src/util/virsystemd.c:288:17: error: implicit declaration of function 'dbus_error_free' [-Werror=implicit-function-declaration] dbus_error_free(&error); Found that this was because commit 318df5a05 needs Dbus libraries for compilation, and configure didnt mark "with_dbus" to be mandatory. In this case, the compilation itself failed in a much uglier fashion on my system where dbus-devel was absent. The following patch introduces a compile error when dbus-devel is absent, so that the build itself proceeds later without cryptic errors. This has also been reported on the list : https://www.redhat.com/archives/libvir-list/2015-January/msg00641.html >From 8c4f583b6bb47ca41866ff884af0cd55487f047d Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Sun, 25 Jan 2015 05:35:23 -0600 Subject: [PATCH] Build: Fix dbus m4 macro to correctly flag it as a required dependency. Commit 318df5a needs dbus headers to compile. However, this package is listed as optional, and so this breaks libvirt compilation on systems that lack the relevant devel files. This patch does not allow ./configure to proceed if dbus-devel is absent. It also tweaks the libvirt spec file to reflect this relationship. --- libvirt.spec.in | 3 --- m4/virt-dbus.m4 | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index ba1cf41..4dfda13 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -627,10 +627,7 @@ BuildRequires: util-linux BuildRequires: nfs-utils %endif -%if %{with_firewalld} -# Communication with the firewall daemon uses DBus BuildRequires: dbus-devel -%endif # Fedora build root suckage BuildRequires: gawk diff --git a/m4/virt-dbus.m4 b/m4/virt-dbus.m4 index 3f9b306..42359cf 100644 --- a/m4/virt-dbus.m4 +++ b/m4/virt-dbus.m4 @@ -19,6 +19,7 @@ dnl AC_DEFUN([LIBVIRT_CHECK_DBUS],[ LIBVIRT_CHECK_PKG([DBUS], [dbus-1], [1.0.0]) + m4_divert_text([DEFAULTS], [with_dbus=yes]) if test "$with_dbus" = "yes" ; then old_CFLAGS="$CFLAGS" -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Firewall : let libvirtd proceed after verifying locking args
Ping ! On Friday 26 December 2014 03:54 PM, Prerna Saxena wrote: > I recently encountered a situation where an unclean ebtables shutdown > caused /var/lib/ebtables/lock to be left behind. When libvirtd was > started on such a system, it caused libvirtd to "hang". Reason: > While probing to check if locking was supported, libvirt runs this > command synchronously : > # /usr/sbin/ebtables --concurrent -L > And this seemed to go on with msgs like : > Trying to obtain lock /var/lib/ebtables/lock > Trying to obtain lock /var/lib/ebtables/lock > Trying to obtain lock /var/lib/ebtables/lock > > Result: > Libvirtd never recovered from this scenario, and the system was > essentially unable to start any VMs. > > The following patch fixes this scenario: > --- > >From ec245eccc03e8a69dc2c2e6edbf30a7b34eb74d0 Mon Sep 17 00:00:00 2001 > From: Prerna Saxena > Date: Fri, 26 Dec 2014 15:24:45 -0500 > Subject: [PATCH] Firewall : let libvirtd proceed after verifying valid locking > args. > > Commit dc33e6e4a5a5d42 introduces locking args to be run with [eb/ip]tables to > determine whether locking is supported. However, this command needs to be > run asynchronously ( as against its present synchronous run), and needs to be > gracefully terminated once the job is done. Else it can potentially stall > libvirtd > with messages like : > "Trying to acquire lock ..." > > Signed-off-by: Prerna Saxena > --- > src/util/virfirewall.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c > index b536912..c120717 100644 > --- a/src/util/virfirewall.c > +++ b/src/util/virfirewall.c > @@ -121,12 +121,16 @@ virFirewallCheckUpdateLock(bool *lockflag, > { > int status; /* Ignore failed commands without logging them */ > virCommandPtr cmd = virCommandNewArgs(args); > -if (virCommandRun(cmd, &status) < 0 || status) { > +status = virCommandRunAsync(cmd, NULL); > +if (status < 0) { > VIR_INFO("locking not supported by %s", args[0]); > +goto cleanup; > } else { > VIR_INFO("using locking for %s", args[0]); > *lockflag = true; > } > +cleanup: > +virCommandAbort(cmd); > virCommandFree(cmd); > } > -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Firewall : let libvirtd proceed after verifying locking args
I recently encountered a situation where an unclean ebtables shutdown caused /var/lib/ebtables/lock to be left behind. When libvirtd was started on such a system, it caused libvirtd to "hang". Reason: While probing to check if locking was supported, libvirt runs this command synchronously : # /usr/sbin/ebtables --concurrent -L And this seemed to go on with msgs like : Trying to obtain lock /var/lib/ebtables/lock Trying to obtain lock /var/lib/ebtables/lock Trying to obtain lock /var/lib/ebtables/lock Result: Libvirtd never recovered from this scenario, and the system was essentially unable to start any VMs. The following patch fixes this scenario: --- >From ec245eccc03e8a69dc2c2e6edbf30a7b34eb74d0 Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Fri, 26 Dec 2014 15:24:45 -0500 Subject: [PATCH] Firewall : let libvirtd proceed after verifying valid locking args. Commit dc33e6e4a5a5d42 introduces locking args to be run with [eb/ip]tables to determine whether locking is supported. However, this command needs to be run asynchronously ( as against its present synchronous run), and needs to be gracefully terminated once the job is done. Else it can potentially stall libvirtd with messages like : "Trying to acquire lock ..." Signed-off-by: Prerna Saxena --- src/util/virfirewall.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index b536912..c120717 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -121,12 +121,16 @@ virFirewallCheckUpdateLock(bool *lockflag, { int status; /* Ignore failed commands without logging them */ virCommandPtr cmd = virCommandNewArgs(args); -if (virCommandRun(cmd, &status) < 0 || status) { +status = virCommandRunAsync(cmd, NULL); +if (status < 0) { VIR_INFO("locking not supported by %s", args[0]); +goto cleanup; } else { VIR_INFO("using locking for %s", args[0]); *lockflag = true; } +cleanup: +virCommandAbort(cmd); virCommandFree(cmd); } -- 1.8.4.2 Regards, -- Prerna Saxena, IBM Systems & technology Labs, Bangalore -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Tests : Make nwfilter testcases robust against optional locking flags.
On Wednesday 26 November 2014 04:06 PM, Martin Kletzander wrote: > On Wed, Nov 26, 2014 at 03:28:21PM +0530, Prerna Saxena wrote: >> Hi Martin, >> Thanks for the feedback. >> >> On Tuesday 25 November 2014 05:57 PM, Martin Kletzander wrote: >>> On Tue, Nov 25, 2014 at 05:12:48PM +0530, Prerna Saxena wrote: >>>> Commit dc33e6e4a5a5d42 introduces iptables/ebtables to adding locking >>>> flags if/when these are available. However, the nwfilter testcases >>>> list outputs without taking into account whether locking flags have been >>>> passed. >>>> >>>> This shows up false testcase failures such as : >>>> 2) ebiptablesTearOldRules... >>>> Offset 1035 >>>> Expect [t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0 >>>> ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0 >>>> ebtables -t nat -L libvirt-I-vnet0 >>>> ebtables -t nat -L libvirt-O-vnet0 >>>> ...snip...] >>>> Actual [-concurrent -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0 >>>> ebtables --concurrent -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0 >>>> ebtables --concurrent -t nat -L libvirt-I-vnet0 >>>> ebtables --concurrent -t nat -L libvirt-O-vnet0 >>>> ...snip...] >>>> >>>> This scrubs all reference to locking flags from test results buffer, >>>> so that achieved output matches the expected results. >>>> >>> Instead of parsing and re-creating the string (which also doesn't >>> check whether we use the locking flag properly), >> The function virtTestClearLockingArgs() merely replaces instances of >> 'ebtables --concurrent' with 'ebtables'. >> (likewise for iptables and ip6tables), if at all found. I didnt find the >> need for sanity checking in this approach :-) >>> it would be way >>> better if we could unify the result. >> Having said so, I agree with this. >> >>> From the top of my head, we can either expose the >>> virFirewallCheckUpdateLock() as non-static and mock it in tests to >>> always set the lock flags to true or we can create new functions that >>> will override setting of the flags. >>> >> The problem is with expected results that are coded for these tests. >> On distros that support these flags, the issue would go away if the expected >> results take into account the locking flags. However, adding a permanent >> change to the expected args string would break >> older distros. > Actually, no, I wanted to unconditionally add the parameters there > only for tests. > > Looking at it more closely, this can fail only if you are building as > root, is that correct? Yes, that is correct. >> So I thought of tweaking the actual results. >> >> Approach #2: >> We could change the expected results to look somewhat like this : >> ebtables $FLAGS -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0 >> >> And have a script that dynamically replaces $FLAGS with : >> * "--concurrent", if locking is supported at compile-time. >> * OR, with " ", if locking is not available. >> >> Ofcourse, not all tests have their expected results in a separate file. Some >> such as >> tests/nwfilterebiptablestest.c have their expected args in the form of char* >> encoded in the same program. This complicates this approach.. >> >> I am looking forward to community suggestions on how this can best be >> implemented. Will be happy to rework this patch if needed. >> >> Regards, >> >> -- >> Prerna Saxena >> >> Linux Technology Centre, >> IBM Systems and Technology Lab, >> Bangalore, India >> -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Tests : Make nwfilter testcases robust against optional locking flags.
Hi Martin, Thanks for the feedback. On Tuesday 25 November 2014 05:57 PM, Martin Kletzander wrote: > On Tue, Nov 25, 2014 at 05:12:48PM +0530, Prerna Saxena wrote: >> Commit dc33e6e4a5a5d42 introduces iptables/ebtables to adding locking >> flags if/when these are available. However, the nwfilter testcases >> list outputs without taking into account whether locking flags have been >> passed. >> >> This shows up false testcase failures such as : >> 2) ebiptablesTearOldRules... >> Offset 1035 >> Expect [t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0 >> ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0 >> ebtables -t nat -L libvirt-I-vnet0 >> ebtables -t nat -L libvirt-O-vnet0 >> ...snip...] >> Actual [-concurrent -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0 >> ebtables --concurrent -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0 >> ebtables --concurrent -t nat -L libvirt-I-vnet0 >> ebtables --concurrent -t nat -L libvirt-O-vnet0 >> ...snip...] >> >> This scrubs all reference to locking flags from test results buffer, >> so that achieved output matches the expected results. >> > Instead of parsing and re-creating the string (which also doesn't > check whether we use the locking flag properly), The function virtTestClearLockingArgs() merely replaces instances of 'ebtables --concurrent' with 'ebtables'. (likewise for iptables and ip6tables), if at all found. I didnt find the need for sanity checking in this approach :-) > it would be way > better if we could unify the result. Having said so, I agree with this. > From the top of my head, we can either expose the > virFirewallCheckUpdateLock() as non-static and mock it in tests to > always set the lock flags to true or we can create new functions that > will override setting of the flags. > The problem is with expected results that are coded for these tests. On distros that support these flags, the issue would go away if the expected results take into account the locking flags. However, adding a permanent change to the expected args string would break older distros. So I thought of tweaking the actual results. Approach #2: We could change the expected results to look somewhat like this : ebtables $FLAGS -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0 And have a script that dynamically replaces $FLAGS with : * "--concurrent", if locking is supported at compile-time. * OR, with " ", if locking is not available. Ofcourse, not all tests have their expected results in a separate file. Some such as tests/nwfilterebiptablestest.c have their expected args in the form of char* encoded in the same program. This complicates this approach.. I am looking forward to community suggestions on how this can best be implemented. Will be happy to rework this patch if needed. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] tests : Fix failure reporting for tests/nwfilterebiptablestest.c
Tests run with 'make check' generally report their results as : Expected: ... Actual: ... 'nwfilterebiptablestest' reports its outcome in opposite sequence, which is confusing for an end-user. This changes 'nwfilterebpitablestest' to report results in a consistent fashion. Signed-off-by: Prerna Saxena --- tests/nwfilterebiptablestest.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c index e04bc21..f62e046 100644 --- a/tests/nwfilterebiptablestest.c +++ b/tests/nwfilterebiptablestest.c @@ -114,8 +114,8 @@ testNWFilterEBIPTablesAllTeardown(const void *opaque ATTRIBUTE_UNUSED) actual = virBufferContentAndReset(&buf); virtTestClearCommandPath(actual); -if (STRNEQ_NULLABLE(actual, expected)) { -virtTestDifference(stderr, actual, expected); +if (STRNEQ_NULLABLE(expected, actual)) { +virtTestDifference(stderr, expected, actual); goto cleanup; } @@ -185,8 +185,8 @@ testNWFilterEBIPTablesTearOldRules(const void *opaque ATTRIBUTE_UNUSED) actual = virBufferContentAndReset(&buf); virtTestClearCommandPath(actual); -if (STRNEQ_NULLABLE(actual, expected)) { -virtTestDifference(stderr, actual, expected); +if (STRNEQ_NULLABLE(expected, actual)) { +virtTestDifference(stderr, expected, actual); goto cleanup; } @@ -234,8 +234,8 @@ testNWFilterEBIPTablesRemoveBasicRules(const void *opaque ATTRIBUTE_UNUSED) actual = virBufferContentAndReset(&buf); virtTestClearCommandPath(actual); -if (STRNEQ_NULLABLE(actual, expected)) { -virtTestDifference(stderr, actual, expected); +if (STRNEQ_NULLABLE(expected, actual)) { +virtTestDifference(stderr, expected, actual); goto cleanup; } @@ -268,8 +268,8 @@ testNWFilterEBIPTablesTearNewRules(const void *opaque ATTRIBUTE_UNUSED) actual = virBufferContentAndReset(&buf); virtTestClearCommandPath(actual); -if (STRNEQ_NULLABLE(actual, expected)) { -virtTestDifference(stderr, actual, expected); +if (STRNEQ_NULLABLE(expected, actual)) { +virtTestDifference(stderr, expected, actual); goto cleanup; } @@ -340,8 +340,8 @@ testNWFilterEBIPTablesApplyBasicRules(const void *opaque ATTRIBUTE_UNUSED) actual = virBufferContentAndReset(&buf); virtTestClearCommandPath(actual); -if (STRNEQ_NULLABLE(actual, expected)) { -virtTestDifference(stderr, actual, expected); +if (STRNEQ_NULLABLE(expected, actual)) { +virtTestDifference(stderr, expected, actual); goto cleanup; } @@ -430,8 +430,8 @@ testNWFilterEBIPTablesApplyDHCPOnlyRules(const void *opaque ATTRIBUTE_UNUSED) actual = virBufferContentAndReset(&buf); virtTestClearCommandPath(actual); -if (STRNEQ_NULLABLE(actual, expected)) { -virtTestDifference(stderr, actual, expected); +if (STRNEQ_NULLABLE(expected, actual)) { +virtTestDifference(stderr, expected, actual); goto cleanup; } @@ -503,8 +503,8 @@ testNWFilterEBIPTablesApplyDropAllRules(const void *opaque ATTRIBUTE_UNUSED) actual = virBufferContentAndReset(&buf); virtTestClearCommandPath(actual); -if (STRNEQ_NULLABLE(actual, expected)) { -virtTestDifference(stderr, actual, expected); +if (STRNEQ_NULLABLE(expected, actual)) { +virtTestDifference(stderr, expected, actual); goto cleanup; } -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Tests : Make nwfilter testcases robust against optional locking flags.
Commit dc33e6e4a5a5d42 introduces iptables/ebtables to adding locking flags if/when these are available. However, the nwfilter testcases list outputs without taking into account whether locking flags have been passed. This shows up false testcase failures such as : 2) ebiptablesTearOldRules... Offset 1035 Expect [t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0 ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0 ebtables -t nat -L libvirt-I-vnet0 ebtables -t nat -L libvirt-O-vnet0 ...snip...] Actual [-concurrent -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0 ebtables --concurrent -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0 ebtables --concurrent -t nat -L libvirt-I-vnet0 ebtables --concurrent -t nat -L libvirt-O-vnet0 ...snip...] This scrubs all reference to locking flags from test results buffer, so that achieved output matches the expected results. Signed-off-by: Prerna Saxena --- tests/nwfilterebiptablestest.c | 7 + tests/nwfilterxml2firewalltest.c | 3 ++ tests/testutils.c| 62 tests/testutils.h| 6 4 files changed, 78 insertions(+) diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c index f62e046..8bf7d53 100644 --- a/tests/nwfilterebiptablestest.c +++ b/tests/nwfilterebiptablestest.c @@ -112,6 +112,7 @@ testNWFilterEBIPTablesAllTeardown(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; actual = virBufferContentAndReset(&buf); +virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES); virtTestClearCommandPath(actual); if (STRNEQ_NULLABLE(expected, actual)) { @@ -183,6 +184,7 @@ testNWFilterEBIPTablesTearOldRules(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; actual = virBufferContentAndReset(&buf); +virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES); virtTestClearCommandPath(actual); if (STRNEQ_NULLABLE(expected, actual)) { @@ -232,6 +234,7 @@ testNWFilterEBIPTablesRemoveBasicRules(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; actual = virBufferContentAndReset(&buf); +virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES); virtTestClearCommandPath(actual); if (STRNEQ_NULLABLE(expected, actual)) { @@ -266,6 +269,7 @@ testNWFilterEBIPTablesTearNewRules(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; actual = virBufferContentAndReset(&buf); +virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES); virtTestClearCommandPath(actual); if (STRNEQ_NULLABLE(expected, actual)) { @@ -338,6 +342,7 @@ testNWFilterEBIPTablesApplyBasicRules(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; actual = virBufferContentAndReset(&buf); +virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES); virtTestClearCommandPath(actual); if (STRNEQ_NULLABLE(expected, actual)) { @@ -428,6 +433,7 @@ testNWFilterEBIPTablesApplyDHCPOnlyRules(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; actual = virBufferContentAndReset(&buf); +virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES); virtTestClearCommandPath(actual); if (STRNEQ_NULLABLE(expected, actual)) { @@ -501,6 +507,7 @@ testNWFilterEBIPTablesApplyDropAllRules(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; actual = virBufferContentAndReset(&buf); +virtTestClearLockingArgs(actual, VIR_TEST_EBTABLES); virtTestClearCommandPath(actual); if (STRNEQ_NULLABLE(expected, actual)) { diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 01527f4..b8c895c 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -402,6 +402,9 @@ static int testCompareXMLToArgvFiles(const char *xml, goto cleanup; actualargv = virBufferContentAndReset(&buf); +virtTestClearLockingArgs(actualargv, VIR_TEST_IPTABLES); +virtTestClearLockingArgs(actualargv, VIR_TEST_IP6TABLES); +virtTestClearLockingArgs(actualargv, VIR_TEST_EBTABLES); virtTestClearCommandPath(actualargv); virCommandSetDryRun(NULL, NULL, NULL); diff --git a/tests/testutils.c b/tests/testutils.c index 9a79f98..fbb41b6 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -930,6 +930,68 @@ void virtTestClearCommandPath(char *cmdset) cmdset[offset] = '\0'; } +/* + * Scrub all reference to arguments that allow iptables to be locked. + * This is a newer iptables change that is unavailable with older distros. + * For seamless comparison of test results between 'expected' &'actual' values, + * it makes sense to *not* compare : + * EXPECTED : /path/to/ebtables -ARG1 -ARG2 -ARG3 + * vs + * ACTUAL : /path/to/iptables --LOCKFLAG -ARG1 -ARG2 -ARG3 + */ +void virtTestClearLockingArgs(char *cmdset, virTestNwFilter filter) +{ +const char *iptables_str = "iptables -w"; +
[libvirt] [PATCH 0/2] Testcase fixes for nwfilter.
This series adds fixes for nwfilter testcases. Patch 1/2 : Flips result reporting for tests/nwfilterebiptablestest.c in line with other tests. Patch 2/2 : Makes nwfilter testcases more resilient to the presence of locking flags, introduced by Commit dc33e6e4a5a5d42 Prerna Saxena (2): tests : Fix failure reporting for tests/nwfilterebiptablestest.c Tests : Make nwfilter testcases robust against optional locking flags. tests/nwfilterebiptablestest.c | 35 ++- tests/nwfilterxml2firewalltest.c | 3 ++ tests/testutils.c| 62 tests/testutils.h| 6 4 files changed, 92 insertions(+), 14 deletions(-) Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] Numa : Allow specification of memory units
On Monday 10 November 2014 07:36 PM, Michal Privoznik wrote: > On 10.11.2014 12:49, Prerna Saxena wrote: >> Present XML specification of NUMA node accepts memory in 'KiB' only. >> This series adds support for input of cell memory in units of choice. >> >> Description: >> === >> Patch 1/2 : Export virDomainParseMemory for use outside domain_conf >> Patch 2/2 : Allow specification of 'units' for NUMA nodes. >> >> Reference: >> == >> v1: http://www.spinics.net/linux/fedora/libvir/msg105516.html >> v2: http://www.mail-archive.com/libvir-list@redhat.com/msg104577.html >> >> Changes Since v2: >> === >> * Patch 1 is newly introduced. It follows Michal's suggestion of reusing >> virDomainParseMemory, exposed by 01b4de2b9f5ca82 >> * Patch 2 : Now uses virDomainParseMemory() >> * Also, documentation in patch 2 is now fixed to link to memory unit >> specification >> > > I've fixed the issues and pushed this. > Thanks, I'll make sure I remember these points in the subsequent libvirt patches I send :-) -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/2] Numa : Allow specification of 'units' for numa nodes.
>From 1ead736712eec3bd098daf222a872c67b67e94ce Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 3 Nov 2014 07:53:59 +0530 CPU numa topology implicitly allows memory specification in 'KiB'. Enabling this to accept the 'unit' in which memory needs to be specified. This now allows users to specify memory in units of choice, and lists the same in 'KiB' -- just like other 'memory' elements in XML. Also augment test cases to correctly model NUMA memory specification. This adds the tag 'unit="KiB"' for memory attribute in NUMA cells. Signed-off-by: Prerna Saxena --- docs/formatdomain.html.in | 8 +++- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c| 44 ++ .../qemuxml2argv-cpu-numa-disjoint.xml | 4 +- .../qemuxml2argv-cpu-numa-memshared.xml| 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 4 +- .../qemuxml2argv-hugepages-pages.xml | 8 ++-- .../qemuxml2argv-hugepages-pages2.xml | 4 +- .../qemuxml2argv-hugepages-pages3.xml | 4 +- .../qemuxml2argv-hugepages-pages4.xml | 8 ++-- .../qemuxml2argv-hugepages-shared.xml | 8 ++-- .../qemuxml2argv-numatune-auto-prefer.xml | 2 +- .../qemuxml2argv-numatune-memnode-no-memory.xml| 4 +- .../qemuxml2argv-numatune-memnode.xml | 6 +-- .../qemuxml2argv-numatune-memnodes-problematic.xml | 4 +- .../qemuxml2xmlout-cpu-numa1.xml | 4 +- .../qemuxml2xmlout-cpu-numa2.xml | 4 +- .../qemuxml2xmlout-numatune-auto-prefer.xml| 2 +- .../qemuxml2xmlout-numatune-memnode.xml| 6 +-- 21 files changed, 81 insertions(+), 60 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7196e75..f103a13 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1153,8 +1153,8 @@ <cpu> ... <numa> - <cell id='0' cpus='0-3' memory='512000'/> - <cell id='1' cpus='4-7' memory='512000' memAccess='shared'/> + <cell id='0' cpus='0-3' memory='512000' unit='KiB'/> + <cell id='1' cpus='4-7' memory='512000' unit='KiB' memAccess='shared'/> </numa> ... </cpu> @@ -1165,6 +1165,10 @@ cpus specifies the CPU or range of CPUs that are part of the node. memory specifies the node memory in kibibytes (i.e. blocks of 1024 bytes). + Since 1.2.11 one can specify an additional + unit attribute to describe the node memory unit. + The detailed syntax for allocation of memory units follows: + unit Since 1.2.7 all cells should have id attribute in case referring to some cell is necessary in the code, otherwise the cells are diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 20d81ae..44cabad 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4144,6 +4144,11 @@ + + + + + shared diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 1c74c66..d0323b0 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -177,6 +177,30 @@ virCPUDefCopy(const virCPUDef *cpu) return NULL; } +static int +virCPUNumaCellMemoryParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned long long* cellMemory) +{ +int ret = -1; +xmlNodePtr oldnode = ctxt->node; + +ctxt->node = node; + +if (virDomainParseMemory("./@memory", "./@unit", ctxt, + cellMemory, true, false) < 0) { +virReportError(VIR_ERR_XML_DETAIL, "%s", + _("Unable to parse NUMA memory size attribute")); +goto cleanup; +} + +ret = 0; + cleanup: +ctxt->node = oldnode; +return ret; + +} + virCPUDefPtr virCPUDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -440,7 +464,7 @@ virCPUDefParseXML(xmlNodePtr node, def->ncells = n; for (i = 0; i < n; i++) { -char *cpus, *memory, *memAccessStr; +char *cpus, *memAccessStr; int ret, ncpus = 0; unsigned int cur_cell; char *tmp = NULL; @@ -489,21 +513,8 @@ virCPUDefParseXML(xmlNodePtr node, goto error; def->cells_cpus += ncpus; -memory = virXMLPropStri
[libvirt] [PATCH v3 1/2] Extend virDomainParseMemory for use outside domain_conf
>From 4978c8c2df19bdf738695d6cc64864f11071a08e Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 10 Nov 2014 14:48:03 +0530 Commit 01b4de2b9f5ca82 abstracts virDomainParseMemory() for use by other functions in domain_conf.c Extend the same for use, for functions outside of this file. Signed-off-by: Prerna Saxena --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 8 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e8d8f7d..5909655 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6421,7 +6421,7 @@ virDomainParseScaledValue(const char *xpath, * * Return 0 on success, -1 on failure after issuing error. */ -static int +int virDomainParseMemory(const char *xpath, const char *units_xpath, xmlXPathContextPtr ctxt, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fbb3b2f..9fb05c8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2847,6 +2847,14 @@ int virDomainObjSetMetadata(virDomainObjPtr vm, const char *configDir, unsigned int flags); +int +virDomainParseMemory(const char *xpath, + const char *units_xpath, + xmlXPathContextPtr ctxt, + unsigned long long *mem, + bool required, + bool capped); + bool virDomainDefNeedsPlacementAdvice(virDomainDefPtr def) ATTRIBUTE_NONNULL(1); -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/2] Numa : Allow specification of memory units
Present XML specification of NUMA node accepts memory in 'KiB' only. This series adds support for input of cell memory in units of choice. Description: === Patch 1/2 : Export virDomainParseMemory for use outside domain_conf Patch 2/2 : Allow specification of 'units' for NUMA nodes. Reference: == v1: http://www.spinics.net/linux/fedora/libvir/msg105516.html v2: http://www.mail-archive.com/libvir-list@redhat.com/msg104577.html Changes Since v2: === * Patch 1 is newly introduced. It follows Michal's suggestion of reusing virDomainParseMemory, exposed by 01b4de2b9f5ca82 * Patch 2 : Now uses virDomainParseMemory() * Also, documentation in patch 2 is now fixed to link to memory unit specification -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Memory : Allow specification of 'units' for numa nodes.
Reference : === v1: http://www.spinics.net/linux/fedora/libvir/msg105516.html Changes since v1: === 1) As suggested by Michal, a new function "virCPUNumaCellMemoryParseXML" has been introduced for neat computation of NUMA memory. 2) Patches 2 & 3 of v1 have been merged together into this cumulative patch. 3) Corresponding documentation added. >From 2199d97b88cf9eab29788fb0e440c4b0a0bb23ec Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 3 Nov 2014 07:53:59 +0530 CPU numa topology implicitly allows memory specification in 'KiB'. Enabling this to accept the 'unit' in which memory needs to be specified. This now allows users to specify memory in units of choice, and lists the same in 'KiB' -- just like other 'memory' elements in XML. Also augment test cases to correctly model NUMA memory specification. This adds the tag 'unit="KiB"' for memory attribute in NUMA cells. Signed-off-by: Prerna Saxena --- docs/formatdomain.html.in | 6 ++- docs/schemas/domaincommon.rng | 5 ++ src/conf/cpu_conf.c| 62 -- .../qemuxml2argv-cpu-numa-disjoint.xml | 4 +- .../qemuxml2argv-cpu-numa-memshared.xml| 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 4 +- .../qemuxml2argv-hugepages-pages.xml | 8 +-- .../qemuxml2argv-hugepages-pages2.xml | 4 +- .../qemuxml2argv-hugepages-pages3.xml | 4 +- .../qemuxml2argv-hugepages-pages4.xml | 8 +-- .../qemuxml2argv-hugepages-shared.xml | 8 +-- .../qemuxml2argv-numatune-auto-prefer.xml | 2 +- .../qemuxml2argv-numatune-memnode-no-memory.xml| 4 +- .../qemuxml2argv-numatune-memnode.xml | 6 +-- .../qemuxml2argv-numatune-memnodes-problematic.xml | 4 +- .../qemuxml2xmlout-cpu-numa1.xml | 4 +- .../qemuxml2xmlout-cpu-numa2.xml | 4 +- .../qemuxml2xmlout-numatune-auto-prefer.xml| 2 +- .../qemuxml2xmlout-numatune-memnode.xml| 6 +-- 21 files changed, 97 insertions(+), 60 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0099ce7..24afc87 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1140,8 +1140,8 @@ <cpu> ... <numa> - <cell id='0' cpus='0-3' memory='512000'/> - <cell id='1' cpus='4-7' memory='512000' memAccess='shared'/> + <cell id='0' cpus='0-3' memory='512000' unit='KiB'/> + <cell id='1' cpus='4-7' memory='512000' unit='KiB' memAccess='shared'/> </numa> ... </cpu> @@ -1152,6 +1152,8 @@ cpus specifies the CPU or range of CPUs that are part of the node. memory specifies the node memory in kibibytes (i.e. blocks of 1024 bytes). + Since 1.2.11 one can specify an additional + unit attribute to describe the node memory unit. Since 1.2.7 all cells should have id attribute in case referring to some cell is necessary in the code, otherwise the cells are diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 20d81ae..44cabad 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4144,6 +4144,11 @@ + + + + + shared diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 5475c07..a0a60c8 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -177,6 +177,48 @@ virCPUDefCopy(const virCPUDef *cpu) return NULL; } +static int +virCPUNumaCellMemoryParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned long long* cellMemory) +{ +int ret = -1; +xmlNodePtr oldnode = ctxt->node; +unsigned long long bytes, max; +char *unit = NULL; + +ctxt->node = node; + +/* 32 vs 64 bit will differ in value of upper memory bound. + * On 32-bit machines, our bound is 0x * KiB. On 64-bit + * machines, our bound is off_t (2^63). + */ +if (sizeof(unsigned long) < sizeof(long long)) +max = 1024ull * ULONG_MAX; +else +max = LLONG_MAX; + +if (virXPathULongLong("string(./@memory)", ctxt, &bytes) < 0) { +virReportError(VIR_ERR_XML_DETAIL, "%s", + _("unable to parse memory size attribute")); +goto cleanup; +} + +unit = virXPathString("string(./@unit)&q
Re: [libvirt] [PATCH 0/3] Libvirt memory & NUMA fixes
On Wednesday 05 November 2014 08:40 PM, Michal Privoznik wrote: > On 05.11.2014 11:56, Prerna Saxena wrote: >> This patch set addresses a bunch of memory & NUMA fixes. >> >> >> Series Description: >> === >> Patch 1/3 : Use consistent data type to represent memory elements in various >> XML attributes. This ensures all memory elements are always represented as >> 'unsigned long long'. >> Patch 2/3 : This adds a 'unit' attribute alongwith the 'memory' attribute of >> a NUMA cell. This enables users to easily describe how much memory needs to >> be allocated to each NUMA cell for a guest >> domain. >> Patch 3/3 : This augments test cases to add the 'unit' tag. >> >> Regards, >> > The 2/3 and 3/3 should be merged together so that 'make check' won't fail > after 2/3. Otherwise looking good. I've merged 1/3 already. > Thanks Michal ! I'll merge the remaining two and send out a cumulative patch :) Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Numa : Allow specification of 'units' for numa nodes.
>From 7fe8487c5e8d24086637d2157bad25322b3654f7 Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 3 Nov 2014 07:53:59 +0530 CPU numa topology implicitly allows memory specification in 'KiB'. Enabling this to accept the 'unit' in which memory needs to be specified. This now allows users to specify memory in units of choice, and lists the same in 'KiB' -- just like other 'memory' elements in XML. I wanted to use virDomainParseScaledValue(), but that function implicitly assumes an XML layout where 'memory' is an _element_ of type 'scaledInteger', with 'unit' as its attribute. A NUMA cell has XML specification where 'memory' is an _attribute_ of element 'cell'. Since changing XML layout is not an option, I have borrowed code from the same. Looking forward to suggestions on how this can best be done. Signed-off-by: Prerna Saxena --- docs/schemas/domaincommon.rng | 5 + src/conf/cpu_conf.c | 36 ++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 20d81ae..44cabad 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4144,6 +4144,11 @@ + + + + + shared diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 5475c07..65b9815 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -189,6 +189,7 @@ virCPUDefParseXML(xmlNodePtr node, char *cpuMode; char *fallback = NULL; char *vendor_id = NULL; +unsigned long long max; if (!xmlStrEqual(node->name, BAD_CAST "cpu")) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -439,10 +440,20 @@ virCPUDefParseXML(xmlNodePtr node, def->ncells = n; +/* 32 vs 64 bit will differ in value of upper memory bound. + * On 32-bit machines, our bound is 0x * KiB. On 64-bit + * machines, our bound is off_t (2^63). + */ +if (sizeof(unsigned long) < sizeof(long long)) +max = 1024ull * ULONG_MAX; +else +max = LLONG_MAX; + for (i = 0; i < n; i++) { -char *cpus, *memory, *memAccessStr; +char *cpus, *memory, *memAccessStr, *unit; int ret, ncpus = 0; unsigned int cur_cell; +unsigned long long bytes; char *tmp = NULL; tmp = virXMLPropString(nodes[i], "id"); @@ -496,14 +507,34 @@ virCPUDefParseXML(xmlNodePtr node, goto error; } -ret = virStrToLong_ull(memory, NULL, 10, &def->cells[cur_cell].mem); +ret = virStrToLong_ull(memory, NULL, 10, &bytes); if (ret == -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid 'memory' attribute in NUMA cell")); VIR_FREE(memory); goto error; } + +unit = virXMLPropString(nodes[i], "unit"); +if (!unit) { +if (VIR_STRDUP(unit, "KiB") < 0) +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Error processing 'memory' in NUMA cell")); +} + +if (virScaleInteger(&bytes, unit, 1024, max) < 0) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("Error scaling 'memory' in NUMA cell")); +VIR_FREE(memory); +VIR_FREE(unit); +goto error; +} + +def->cells[cur_cell].mem = VIR_DIV_UP(bytes, 1024); + +bytes = 0; VIR_FREE(memory); +VIR_FREE(unit); memAccessStr = virXMLPropString(nodes[i], "memAccess"); if (memAccessStr) { @@ -703,6 +734,7 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAsprintf(buf, " id='%zu'", i); virBufferAsprintf(buf, " cpus='%s'", def->cells[i].cpustr); virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem); +virBufferAddLit(buf, " unit='KiB'"); if (memAccess) virBufferAsprintf(buf, " memAccess='%s'", virMemAccessTypeToString(memAccess)); -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3]Test:Augment test cases to correctly model NUMA specification.
>From 2fe0e329e7224b7cd29e1252e4b4e70d9195ab2b Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 3 Nov 2014 15:16:12 +0530 This adds the tag 'unit="KiB"' for memory attribute in NUMA cells. Signed-off-by: Prerna Saxena --- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml| 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml | 8 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml | 8 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml | 8 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml | 6 +++--- .../qemuxml2argv-numatune-memnodes-problematic.xml| 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml | 6 +++--- 18 files changed, 42 insertions(+), 42 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml index 474a238..bdffcd1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml index cf7c040..c638ffa 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml index 0543f7f..20120e9 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml index 0a5f9fc..a90e7a2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml index fa3070d..ea2dc81 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml index 5ad0695..b67df2f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml @@ -20,10 +20,10 @@ - - - - + + + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml index 3df870b..6afa6ef 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml @@ -15,8 +15,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml index 35aa2cf..21f4985 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml @@ -15,8 +15,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml index a3ed29b..eb18f24 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml @@ -20,10 +20,10 @@ - - - - + + + + diff --git a/tests/qemuxm
[libvirt] [PATCH 1/3] Memory: Use consistent type for all memory elements.
>From 4b3e336ea045759758b04440d75802e990506e2b Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Fri, 31 Oct 2014 16:07:21 +0530 Domain memory elements such as max_balloon and cur_balloon are implemented as 'unsigned long long', whereas the 'memory' element in NUMA cells is implemented as 'unsigned int'. Use the same data type (unsigned long long) for 'memory' element in NUMA cells. Signed-off-by: Prerna Saxena --- src/conf/cpu_conf.c | 4 ++-- src/conf/cpu_conf.h | 2 +- src/qemu/qemu_command.c | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 9b7fbb0..5475c07 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -496,7 +496,7 @@ virCPUDefParseXML(xmlNodePtr node, goto error; } -ret = virStrToLong_ui(memory, NULL, 10, &def->cells[cur_cell].mem); +ret = virStrToLong_ull(memory, NULL, 10, &def->cells[cur_cell].mem); if (ret == -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid 'memory' attribute in NUMA cell")); @@ -702,7 +702,7 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAddLit(buf, "cells[i].cpustr); -virBufferAsprintf(buf, " memory='%d'", def->cells[i].mem); +virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem); if (memAccess) virBufferAsprintf(buf, " memAccess='%s'", virMemAccessTypeToString(memAccess)); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index d45210b..5bcf101 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -105,7 +105,7 @@ typedef virCellDef *virCellDefPtr; struct _virCellDef { virBitmapPtr cpumask; /* CPUs that are part of this node */ char *cpustr; /* CPUs stored in string form for dumpxml */ -unsigned int mem; /* Node memory in kB */ +unsigned long long mem; /* Node memory in kB */ virMemAccess memAccess; }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 917639e..13b54dd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6693,7 +6693,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, } for (i = 0; i < def->cpu->ncells; i++) { -int cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024); +unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024); def->cpu->cells[i].mem = cellmem * 1024; virMemAccess memAccess = def->cpu->cells[i].memAccess; @@ -6799,7 +6799,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virBufferAddLit(&buf, "memory-backend-ram"); } -virBufferAsprintf(&buf, ",size=%dM,id=ram-node%zu", cellmem, i); +virBufferAsprintf(&buf, ",size=%lluM,id=ram-node%zu", cellmem, i); if (virDomainNumatuneMaybeFormatNodeset(def->numatune, nodeset, &nodemask, i) < 0) @@ -6849,7 +6849,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { virBufferAsprintf(&buf, ",memdev=ram-node%zu", i); } else { -virBufferAsprintf(&buf, ",mem=%d", cellmem); +virBufferAsprintf(&buf, ",mem=%llu", cellmem); } virCommandAddArgBuffer(cmd, &buf); -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Libvirt memory & NUMA fixes
This patch set addresses a bunch of memory & NUMA fixes. Series Description: === Patch 1/3 : Use consistent data type to represent memory elements in various XML attributes. This ensures all memory elements are always represented as 'unsigned long long'. Patch 2/3 : This adds a 'unit' attribute alongwith the 'memory' attribute of a NUMA cell. This enables users to easily describe how much memory needs to be allocated to each NUMA cell for a guest domain. Patch 3/3 : This augments test cases to add the 'unit' tag. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v5][Patch 0/4] Libvirt CPU enhancements for Power KVM
On Monday 03 November 2014 02:57 PM, Michal Privoznik wrote: > On 31.10.2014 10:15, Prerna Saxena wrote: >> This patch series is a collection of enhancements for PowerPC CPUs on >> PowerKVM. >> In this iteration, I have followed Dan's suggestion on using existing cpu >> mode >> format to describe powerPC compatibility mode. >> Hope this can finally make it for 1.2.10 ! >> >> Series Summary: >> == >> Patch 1/4 : Introduce a new architecture 'ppc64le' for libvirt. >> Patch 2/4 : Add libvirt support for VMs running in 'compat' mode on Power >> KVM. >> Patch 3/4 : Improve PVR handling to fall back to cpu generation. >> Patch 4/4 : Add documentation describing compat mode usage for PowerPC >> guests. > > What am I missing here is a test case. Can you please add a qemuxml2argv test > case? Just send it as a follow up patch and I'll merge the code then. > > Michal > Hi Michal, I have sent out a v6 of the patches rebased on top of latest master. Apart from introducing a testcase, these have minor changes over v5. Thanks for the review, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 5/5]Test: Add a testcase for PowerPC compat mode cpu specification.
>From 88879d7eac1237b2f6ef67cb5890cb46055d56dc Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 3 Nov 2014 15:53:08 +0530 This introduces a testcase for PowerPC compat mode cpu specification. Signed-off-by: Prerna Saxena --- .../qemuxml2argv-pseries-cpu-compat.args | 8 .../qemuxml2argv-pseries-cpu-compat.xml | 20 tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 30 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args new file mode 100644 index 000..64df406 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +QEMU_AUDIO_DRV=none /usr/bin/qemu-system-ppc64 -S -M pseries \ +-cpu host,compat=power7 \ +-m 214 -smp 4 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb \ +-chardev pty,id=charserial0 \ +-device spapr-vty,chardev=charserial0,reg=0x3000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.xml new file mode 100644 index 000..e34a8ad --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.xml @@ -0,0 +1,20 @@ + + QEMUGuest1 + 219100 + 219100 + 4 + +hvm + + +power7 + + + + /usr/bin/qemu-system-ppc64 + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index abdf516..2a1ca4b 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1308,6 +1308,8 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-cpu-exact", QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +DO_TEST("pseries-cpu-compat", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST, +QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 4/5] docs: Add documentation for compat mode.
>From 23f49711d74fae7905defa1524d7e4ab838c7838 Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Fri, 31 Oct 2014 15:13:16 +0530 Add documentation to explain how compat-mode can be invoked with libvirt running on PowerPC architecture. It also mentions that this change is available libvirt 1.2.11 onwards. Signed-off-by: Prerna Saxena --- docs/formatdomain.html.in | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0099ce7..bdaf808 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1047,7 +1047,20 @@ (such as CPUID level) that don't work. Until these issues are fixed, it's a good idea to avoid using host-model and use custom mode with just the CPU model from host - capabilities XML. + capabilities XML. + (Since 1.2.11). PowerISA allows + processors to run VMs in binary compatibility mode supporting an + older version of ISA. Libvirt on PowerPC architecture uses the + host-model to signify a guest mode CPU running in + binary compatibility mode. Example: + When a user needs a power7 VM to run in compatibility mode + on a Power8 host, this can be described in XML as follows : + + <cpu mode='host-model'> +<model>power7</model> + </cpu> + ... + host-passthrough With this mode, the CPU visible to the guest should be exactly the same as the host CPU even in the aspects that libvirt does not -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 3/5]PowerPC:Improve PVR handling to fall back to cpu generation.
>From eebc1544e28a134ce99d39b663f09ffa89b8064a Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Tue, 28 Oct 2014 15:30:05 +0530 IBM Power processors differ uniquely across generations (such as power6, power7, power8). Each generation signifies a new PowerISA version that exhibits features unique to that generation. The higher 16 bits of PVR for IBM Power processors encode the CPU generation, while the CPU chip (sub)version is encoded in lower 16 bits. For all practical purposes of launching a VM, we care about the generation which the vCPU will belong to, and not specifically the chip version. This patch updates the libvirt PVR check to reflect this relationship. It allows libvirt to select the right CPU generation in case the exact match for a a specific CPU is not found. Hence, there will no longer be a need to add each PowerPC CPU model to cpu_map.xml; just adding entry for the matching ISA generation will suffice. It also contains changes to cpu_map.xml since processor generations as understood by QEMU compat mode go as "power6", "power7" or "power8" [Reference : QEMU commit 8dfa3a5e85 ] Signed-off-by: Prerna Saxena Signed-off-by: Pradipta Kr. Banerjee Signed-off-by: Anton Blanchard Acked-by: Michal Privoznik --- src/cpu/cpu_map.xml | 30 ++ src/cpu/cpu_powerpc.c | 8 2 files changed, 38 insertions(+) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 18c7b0d..bd9b056 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -627,5 +627,35 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 4ea1835..531868c 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -99,6 +99,14 @@ ppcModelFindPVR(const struct ppc_map *map, model = model->next; } +/* PowerPC Processor Version Register is interpreted as follows : + * Higher order 16 bits : Power ISA generation. + * Lower order 16 bits : CPU chip version number. + * If the exact CPU isnt found, return the nearest matching CPU generation + */ +if (pvr & 0xul) +return ppcModelFindPVR(map, (pvr & 0xul)); + return NULL; } -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 2/5] PowerPC : Add support for launching VM in 'compat' mode.
>From 3ad5caf37bfa48e43c88660255e6a3bbb8afddeb Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Tue, 28 Oct 2014 15:05:59 +0530 PowerISA allows processors to run VMs in binary compatibility ("compat") mode supporting an older version of ISA. QEMU has recently added support to explicitly denote a VM running in compatibility mode through commit 6d9412ea & 8dfa3a5e85. Now, a "compat" mode VM can be run by invoking this qemu commandline on a POWER8 host: -cpu host,compat=power7. This patch allows libvirt to exploit cpu mode 'host-model' to describe this new mode for PowerKVM guests. As an example: When a user wants to request a power7 vm to run in compatibility mode on a Power8 host, this can be described in XML as follows : power7 Signed-off-by: Prerna Saxena Signed-off-by: Li Zhang Signed-off-by: Pradipta Kr. Banerjee Acked-by: Michal Privoznik --- src/conf/cpu_conf.c | 1 + src/cpu/cpu_powerpc.c | 11 ++- src/qemu/qemu_command.c | 10 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 9b7fbb0..0e7a979 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -619,6 +619,7 @@ virCPUDefFormatBuf(virBufferPtr buf, return 0; formatModel = (def->mode == VIR_CPU_MODE_CUSTOM || + def->mode == VIR_CPU_MODE_HOST_MODEL || (flags & VIR_DOMAIN_XML_UPDATE_CPU)); formatFallback = (def->type == VIR_CPU_TYPE_GUEST && (def->mode == VIR_CPU_MODE_HOST_MODEL || diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index d591c18..4ea1835 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -562,8 +562,8 @@ ppcUpdate(virCPUDefPtr guest, static virCPUDefPtr ppcBaseline(virCPUDefPtr *cpus, unsigned int ncpus, -const char **models, -unsigned int nmodels, +const char **models ATTRIBUTE_UNUSED, +unsigned int nmodels ATTRIBUTE_UNUSED, unsigned int flags) { struct ppc_map *map = NULL; @@ -583,13 +583,6 @@ ppcBaseline(virCPUDefPtr *cpus, goto error; } -if (!cpuModelIsAllowed(model->name, models, nmodels)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -_("CPU model %s is not supported by hypervisor"), -model->name); -goto error; -} - for (i = 0; i < ncpus; i++) { const struct ppc_vendor *vnd; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 96071d8..1333c35 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6222,7 +6222,9 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, *hasHwVirt = hasSVM > 0 ? true : false; } -if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { +if ((cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) || +((cpu->mode == VIR_CPU_MODE_HOST_MODEL) && + ARCH_IS_PPC64(def->os.arch))) { const char *mode = virCPUModeTypeToString(cpu->mode); if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_HOST)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -6237,6 +6239,12 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, goto cleanup; } virBufferAddLit(buf, "host"); + +if (ARCH_IS_PPC64(def->os.arch) && +cpu->mode == VIR_CPU_MODE_HOST_MODEL) { +virBufferAsprintf(buf, ",compat=%s", def->cpu->model); +} + } else { if (VIR_ALLOC(guest) < 0) goto cleanup; -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 1/5] Cpu: Add support for Power LE Architecture.
>From 0c8b80da2f3ea85f65d5b6a7b841433d5162a3bd Mon Sep 17 00:00:00 2001 From: "Pradipta Kr. Banerjee" Date: Tue, 28 Oct 2014 14:41:59 +0530 Subject: [PATCH 1/5] Cpu: Add support for Power LE Architecture. This adds support for PowerPC Little Endian architecture., and allows libvirt to spawn VMs based on 'ppc64le' architecture. Signed-off-by: Pradipta Kr. Banerjee Signed-off-by: Prerna Saxena Acked-by: Michal Privoznik --- src/conf/domain_conf.c | 2 +- src/cpu/cpu_powerpc.c| 2 +- src/qemu/qemu_capabilities.c | 6 +++--- src/qemu/qemu_command.c | 22 +++--- src/qemu/qemu_domain.c | 1 + src/util/virarch.h | 3 +++ 6 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1b8efb1..21309b0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10043,7 +10043,7 @@ virDomainVideoDefaultType(const virDomainDef *def) (STREQ(def->os.type, "xen") || STREQ(def->os.type, "linux"))) return VIR_DOMAIN_VIDEO_TYPE_XEN; -else if (def->os.arch == VIR_ARCH_PPC64) +else if ARCH_IS_PPC64(def->os.arch) return VIR_DOMAIN_VIDEO_TYPE_VGA; else return VIR_DOMAIN_VIDEO_TYPE_CIRRUS; diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 67cb9ff..d591c18 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -38,7 +38,7 @@ VIR_LOG_INIT("cpu.cpu_powerpc"); -static const virArch archs[] = { VIR_ARCH_PPC64 }; +static const virArch archs[] = { VIR_ARCH_PPC64, VIR_ARCH_PPC64LE }; struct ppc_vendor { char *name; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ec6614a..2505d32 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -633,7 +633,7 @@ virQEMUCapsProbeCPUModels(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid) if (qemuCaps->arch == VIR_ARCH_I686 || qemuCaps->arch == VIR_ARCH_X86_64) { parse = virQEMUCapsParseX86Models; -} else if (qemuCaps->arch == VIR_ARCH_PPC64) { +} else if ARCH_IS_PPC64(qemuCaps->arch) { parse = virQEMUCapsParsePPCModels; } else { VIR_DEBUG("don't know how to parse %s CPU models", @@ -2003,7 +2003,7 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return true; if (def->os.arch == VIR_ARCH_PPC || -def->os.arch == VIR_ARCH_PPC64) { +ARCH_IS_PPC64(def->os.arch)) { /* * Usage of pci.0 naming: * @@ -3575,7 +3575,7 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; -if ((def->os.arch == VIR_ARCH_PPC) || (def->os.arch == VIR_ARCH_PPC64)) { +if ((def->os.arch == VIR_ARCH_PPC) || ARCH_IS_PPC64(def->os.arch)) { /* only pseries need -device spapr-vty with -chardev */ return (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 917639e..96071d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -714,7 +714,7 @@ qemuSetSCSIControllerModel(virDomainDefPtr def, return -1; } } else { -if ((def->os.arch == VIR_ARCH_PPC64) && +if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { @@ -1265,7 +1265,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, for (i = 0; i < def->nserials; i++) { if (def->serials[i]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && -(def->os.arch == VIR_ARCH_PPC64) && +ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, @@ -1274,7 +1274,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, } if (def->nvram) { -if (def->os.arch == VIR_ARCH_PPC64 && +if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuAssignSpaprVIOAddress(def, &def->nvram->info, @@ -4196,7 +4196,7 @@ qemuBuildUSBControllerDevStr(virDomainDefPtr domainDef, model = def->model; if (model == -1) { -i
[libvirt] [Patch v6 0/5] Libvirt CPU enhancements for Power KVM
This patch series is a collection of enhancements for PowerPC CPUs on PowerKVM. The v5 of this series has been acked. I have just rebased the patches on top of latest master and added a testcase. Series Summary: == Patch 1/5 : Introduce a new architecture 'ppc64le' for libvirt. Patch 2/5 : Add libvirt support for VMs running in 'compat' mode on Power KVM. Patch 3/5 : Improve PVR handling to fall back to cpu generation. Patch 4/5 : Add documentation describing compat mode usage for PowerPC guests. Patch 5/5 : Add a test case for compat mode. Detail: * PowerPC has traditionally been a Big-endian architecture. However, with PowerPC ISA version 2.07, it can run in Little-endian mode as well. IBM Power8 processors, compliant with ISA 2.07 allow launching VMs in little-endian mode. This is signified by 'ppc64le' architecture. Patch 1 adds this support to libvirt, to allow running VMs based on ppc64le architecture. * Patch 2,3 tweak libvirt to correctly model PowerPC CPUs based on recent PowerKVM implementation. PowerKVM permits VMs with vcpus in the following allowed modes : i) Host native mode: where the vcpu seen in the VM belongs to the same processor generation as the host. Example: A POWER7 host, conforming to PowerISA version 2.06, will run VMs with "power7" vcpus. ii) Binary Compatibility ("compat") mode: PowerISA allows processors to run VMs in binary compatibility ("compat") mode supporting an older version of ISA. As an example: In compatibility mode, a POWER7 host can run a "power6" VM, conforming to power ISA v2.05. Similarly, a POWER8 host can run a "power7" VM conforming to PowerISA v2.06. QEMU has recently added support to explicitly denote a VM running in compatibility mode through commits 6d9412ea & 8dfa3a5e85. Henceforth, VMs of type (i) will be invoked with the QEMU invocation "-cpu host", while VMs of type (ii) will be invoked using "-cpu host, compat=power6". Now, an explicit cpu selection using "-cpu POWER6" is not valid. Instead, the recommended practice is to use the matching compat mode, if the requested cpu type differs from the host. Patches 2-3 address various aspects of this change. * Patch 2 : Adds support for generating the correct command line for QEMU. Existing xml semantics of 'host-model' are interpreted differently on PowerPc architecture to signify this type. * Patch 3 : PowerKVM vCPUs differ uniquely across generations ( such as power6, power7, power8). Each generation signifies a new PowerISA version that exhibits features unique to that generation. The higher order 16 bits of PVR denote the processor generation and the lower order 16 bits denote the cpu chip (sub)version. For all practical purposes of launching a VM, we care about the generation the vCPU will belong to, and not specifically the chip version. In fact, PowerKVM does not seek out specification of a unique chip version(such as POWER7_v2.3) for running a vCPU. This patch updates the libvirt PVR check to reflect this relationship. * Patch 4 : Documentation is added to explain functionality introduced by Patch 2. * Patch 5 : Added a test case for patch 2. Changelog: = v1 : https://www.redhat.com/archives/libvir-list/2014-June/msg01338.html v2 : http://www.redhat.com/archives/libvir-list/2014-October/msg00351.html v3 : http://www.mail-archive.com/libvir-list@redhat.com/msg104010.html v4 : http://www.mail-archive.com/libvir-list@redhat.com/msg104067.html v5 : http://www.mail-archive.com/libvir-list@redhat.com/msg104311.html Changes since v5: * Added a new patch which introduces a test case for compat mode. * Fixed a whitespace in documentation patch #4. * Added listing for Power8e cpu model in cpu_map.xml Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list