Re: [libvirt] [PATCH 0/2] target-i386: Haswell-noTSX and Broadwell-noTSX CPU models
Am 13.03.2015 um 20:09 schrieb Eduardo Habkost: With the Intel microcode update that removed HLE and RTM, there will be different kinds of Haswell and Broadwell CPUs out there: some that still have the HLE and RTM features, and some that don't have the HLE and RTM features. On both cases people may be willing to use the pc-*-2.3 machine-types. So instead of making the CPU model results confusing by making it depend on the machine-type, keep HLE and RTM on the existing Haswell and Broadwell CPU models, and introduce Haswell-noTSX and Broadwell-noTSX CPU models later, for people who have CPUs that don't have TSX feature available. Eduardo Habkost (2): Revert target-i386: Disable HLE and RTM on Haswell Broadwell target-i386: Haswell-noTSX and Broadwell-noTSX No objections from a generic CPU point of view. Only thing that comes to mind is whether it might make sense to hierarchically make Broadwell the parent type of Broadwell-noTSX, to avoid duplication. But then again we already have a lot of it. ;) Regards, Andreas hw/i386/pc_piix.c | 4 --- hw/i386/pc_q35.c | 4 --- target-i386/cpu.c | 74 +-- 3 files changed, 72 insertions(+), 10 deletions(-) -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Revert target-i386: Disable HLE and RTM on Haswell Broadwell
This reverts commit 13704e4c455770d500d6b87b117e32f0d01252c9. With the Intel microcode update that removed HLE and RTM, there will be different kinds of Haswell and Broadwell CPUs out there: some that still have the HLE and RTM features, and some that don't have the HLE and RTM features. On both cases people may be willing to use the pc-*-2.3 machine-types. So instead of making the CPU model results confusing by making it depend on the machine-type, keep HLE and RTM on the existing Haswell and Broadwell CPU models. The plan is to introduce Haswell-noTSX and Broadwell-noTSX CPU models later, for people who have CPUs that don't have TSX feature available. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- hw/i386/pc_piix.c | 4 hw/i386/pc_q35.c | 4 target-i386/cpu.c | 9 + 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 8eab4ba..0e32afa 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -331,10 +331,6 @@ static void pc_compat_2_2(MachineState *machine) x86_cpu_compat_set_features(Haswell, FEAT_1_ECX, 0, CPUID_EXT_RDRAND); x86_cpu_compat_set_features(Broadwell, FEAT_1_ECX, 0, CPUID_EXT_F16C); x86_cpu_compat_set_features(Broadwell, FEAT_1_ECX, 0, CPUID_EXT_RDRAND); -x86_cpu_compat_set_features(Haswell, FEAT_7_0_EBX, -CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0); -x86_cpu_compat_set_features(Broadwell, FEAT_7_0_EBX, -CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0); } static void pc_compat_2_1(MachineState *machine) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index c0f21fe..5a7b11c 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -310,10 +310,6 @@ static void pc_compat_2_2(MachineState *machine) x86_cpu_compat_set_features(Haswell, FEAT_1_ECX, 0, CPUID_EXT_RDRAND); x86_cpu_compat_set_features(Broadwell, FEAT_1_ECX, 0, CPUID_EXT_F16C); x86_cpu_compat_set_features(Broadwell, FEAT_1_ECX, 0, CPUID_EXT_RDRAND); -x86_cpu_compat_set_features(Haswell, FEAT_7_0_EBX, -CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0); -x86_cpu_compat_set_features(Broadwell, FEAT_7_0_EBX, -CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0); } static void pc_compat_2_1(MachineState *machine) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index ed7e5d5..de3cdce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1099,8 +1099,9 @@ static X86CPUDefinition builtin_x86_defs[] = { CPUID_EXT3_LAHF_LM, .features[FEAT_7_0_EBX] = CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | -CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | -CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID, +CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | +CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID | +CPUID_7_0_EBX_RTM, .features[FEAT_XSAVE] = CPUID_XSAVE_XSAVEOPT, .xlevel = 0x800A, @@ -1133,9 +1134,9 @@ static X86CPUDefinition builtin_x86_defs[] = { CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH, .features[FEAT_7_0_EBX] = CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | -CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | +CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID | -CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX | +CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX | CPUID_7_0_EBX_SMAP, .features[FEAT_XSAVE] = CPUID_XSAVE_XSAVEOPT, -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] target-i386: Haswell-noTSX and Broadwell-noTSX CPU models
With the Intel microcode update that removed HLE and RTM, there will be different kinds of Haswell and Broadwell CPUs out there: some that still have the HLE and RTM features, and some that don't have the HLE and RTM features. On both cases people may be willing to use the pc-*-2.3 machine-types. So instead of making the CPU model results confusing by making it depend on the machine-type, keep HLE and RTM on the existing Haswell and Broadwell CPU models, and introduce Haswell-noTSX and Broadwell-noTSX CPU models later, for people who have CPUs that don't have TSX feature available. Eduardo Habkost (2): Revert target-i386: Disable HLE and RTM on Haswell Broadwell target-i386: Haswell-noTSX and Broadwell-noTSX hw/i386/pc_piix.c | 4 --- hw/i386/pc_q35.c | 4 --- target-i386/cpu.c | 74 +-- 3 files changed, 72 insertions(+), 10 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: process: Export qemuProcessFindDomainDiskByAlias
On 03/13/2015 10:25 AM, Peter Krempa wrote: --- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/9] conf: add virDomainHasNet
Marek Marczykowski-Górecki wrote: virDomainNetFindIdx no longer returns info whether device was not found, or there was multiple matches. Additionally it already handle error reporting. Introduce virDomainHasNet which does a simple task, without implicit error reporting. Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- src/conf/domain_conf.c | 21 + src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 23 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9b7ae3f..c430e03 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11908,6 +11908,27 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net) return matchidx; } +bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net) Nit: libvirt style is to have return type on separate line. +{ +size_t i; +bool PCIAddrSpecified = virDomainDeviceAddressIsValid(net-info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI); + +for (i = 0; i def-nnets; i++) { +if (virMacAddrCmp(def-nets[i]-mac, net-mac)) +continue; + +if (PCIAddrSpecified) { Braces here are fine, where the single statement spans multiple lines. +if (virDevicePCIAddressEqual(def-nets[i]-info.addr.pci, + net-info.addr.pci)) { +return true; +} But not needed here. ACK. Will fix the nits before pushing. Regards, Jim +} else { +return true; +} +} +return false; +} void virDomainNetRemoveHostdev(virDomainDefPtr def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 02ddd93..36c8131 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2601,6 +2601,7 @@ bool virDomainHasDiskMirror(virDomainObjPtr vm); int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net); virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device); +bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net); int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net); virDomainNetDefPtr virDomainNetRemove(virDomainDefPtr def, size_t i); void virDomainNetRemoveHostdev(virDomainDefPtr def, virDomainNetDefPtr net); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ba05cc6..5fe6ae0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -292,6 +292,7 @@ virDomainGraphicsTypeToString; virDomainGraphicsVNCSharePolicyTypeFromString; virDomainGraphicsVNCSharePolicyTypeToString; virDomainHasDiskMirror; +virDomainHasNet; virDomainHostdevCapsTypeToString; virDomainHostdevDefAlloc; virDomainHostdevDefClear; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/9] libxl: prevent attaching multiple netdevs with the same MAC
Marek Marczykowski-Górecki wrote: It will not be possible to detach such device later. Also improve logging in such cases. Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- src/libxl/libxl_driver.c | 15 +++ 1 file changed, 15 insertions(+) Changes in v4: - use virDomainHasNet instead of virDomainNetFindIdx diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ce3a99b..1313d2e 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2787,6 +2787,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, int actualType; libxl_device_nic nic; int ret = -1; +char mac[VIR_MAC_STRING_BUFLEN]; /* preallocate new slot for device */ if (VIR_REALLOC_N(vm-def-nets, vm-def-nnets + 1) 0) @@ -2801,6 +2802,13 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, actualType = virDomainNetGetActualType(net); +if (virDomainHasNet(vm-def, net)) { +virReportError(VIR_ERR_OPERATION_FAILED, +_(device matching mac address %s already exists), + virMacAddrFormat(net-mac, mac)); I think the error should be VIR_ERR_INVALID_ARG, like you've done in the last hunk below. I'll change that, and use the same text in both error reports, before pushing. Otherwise, ACK. Regards, Jim +return -1; +} + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { /* This is really a smart hostdev, so it should be attached * as a hostdev (the hostdev code will reach over into the @@ -2879,6 +2887,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) virDomainHostdevDefPtr hostdev; virDomainHostdevDefPtr found; virDomainHostdevSubsysPCIPtr pcisrc; +char mac[VIR_MAC_STRING_BUFLEN]; switch (dev-type) { case VIR_DOMAIN_DEVICE_DISK: @@ -2896,6 +2905,12 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) case VIR_DOMAIN_DEVICE_NET: net = dev-data.net; +if (virDomainHasNet(vmdef, net)) { +virReportError(VIR_ERR_INVALID_ARG, + _(network device with mac %s already exists.), + virMacAddrFormat(net-mac, mac)); +return -1; +} if (virDomainNetInsert(vmdef, net)) return -1; dev-data.net = NULL; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/9] libxl: support domain config modification in virDomainRestoreFlags
Marek Marczykowski-Górecki wrote: Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- src/libxl/libxl_driver.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 1313d2e..d7f5dac 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1456,11 +1456,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, #endif virCheckFlags(VIR_DOMAIN_SAVE_PAUSED, -1); -if (dxml) { -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, - _(xml modification unsupported)); -return -1; -} fd = libxlDomainSaveImageOpen(driver, cfg, from, def, hdr); if (fd 0) @@ -1469,6 +1464,18 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, if (virDomainRestoreFlagsEnsureACL(conn, def) 0) goto cleanup_unlock; +if (dxml) { +virDomainDefPtr def2 = NULL; + +if (!(def2 = virDomainDefParseString(dxml, cfg-caps, driver-xmlopt, +1 VIR_DOMAIN_VIRT_XEN, +VIR_DOMAIN_XML_INACTIVE))) { +goto cleanup; +} +virDomainDefFree(def); +def = def2; +} + This patch looks good and I'm fine with committing it, even though I do worry a bit about future bug reports where users have changed the config in incompatible ways and the domain fails to restore. As before, I'd like to hear what others have to say. Did the qemu driver experience similar bug reports before checking ABI stability? One possible solution is to have virDomainDefCheckMetaABIStability() and virDomainDefCheckDeviceABIStability() functions. The latter could be used by callers wanting virtual hardware and device ABI stability only. Together, they provide virDomainDefCheckABIStability() for callers wanting strict ABI stability. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] target-i386: Haswell-noTSX and Broadwell-noTSX
With the Intel microcode update that removed HLE and RTM, there will be different kinds of Haswell and Broadwell CPUs out there: some that still have the HLE and RTM features, and some that don't have the HLE and RTM features. On both cases people may be willing to use the pc-*-2.3 machine-types. So, to cover both cases, introduce Haswell-noTSX and Broadwell-noTSX CPU models, for hosts that have Haswell and Broadwell CPUs without TSX support. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/cpu.c | 69 +++ 1 file changed, 69 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index de3cdce..b693bab 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1073,6 +1073,39 @@ static X86CPUDefinition builtin_x86_defs[] = { .model_id = Intel Xeon E3-12xx v2 (Ivy Bridge), }, { +.name = Haswell-noTSX, +.level = 0xd, +.vendor = CPUID_VENDOR_INTEL, +.family = 6, +.model = 60, +.stepping = 1, +.features[FEAT_1_EDX] = +CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX | +CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA | +CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 | +CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE | +CPUID_DE | CPUID_FP87, +.features[FEAT_1_ECX] = +CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES | +CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 | +CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | +CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 | +CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE | +CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND, +.features[FEAT_8000_0001_EDX] = +CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX | +CPUID_EXT2_SYSCALL, +.features[FEAT_8000_0001_ECX] = +CPUID_EXT3_LAHF_LM, +.features[FEAT_7_0_EBX] = +CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | +CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | +CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID, +.features[FEAT_XSAVE] = +CPUID_XSAVE_XSAVEOPT, +.xlevel = 0x800A, +.model_id = Intel Core Processor (Haswell, no TSX), +},{ .name = Haswell, .level = 0xd, .vendor = CPUID_VENDOR_INTEL, @@ -1108,6 +1141,42 @@ static X86CPUDefinition builtin_x86_defs[] = { .model_id = Intel Core Processor (Haswell), }, { +.name = Broadwell-noTSX, +.level = 0xd, +.vendor = CPUID_VENDOR_INTEL, +.family = 6, +.model = 61, +.stepping = 2, +.features[FEAT_1_EDX] = +CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX | +CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA | +CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 | +CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE | +CPUID_DE | CPUID_FP87, +.features[FEAT_1_ECX] = +CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES | +CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 | +CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | +CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 | +CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE | +CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND, +.features[FEAT_8000_0001_EDX] = +CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX | +CPUID_EXT2_SYSCALL, +.features[FEAT_8000_0001_ECX] = +CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH, +.features[FEAT_7_0_EBX] = +CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | +CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | +CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID | +CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX | +CPUID_7_0_EBX_SMAP, +.features[FEAT_XSAVE] = +CPUID_XSAVE_XSAVEOPT, +.xlevel = 0x800A, +.model_id = Intel Core Processor (Broadwell, no TSX), +}, +{ .name = Broadwell, .level = 0xd, .vendor = CPUID_VENDOR_INTEL, -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/9] xenconfig: add support for multiple USB devices syntax
Marek Marczykowski-Górecki wrote: In Xen=4.3, libxl supports new syntax for USB devices: usbdevice=[ DEVICE, DEVICE, ... ] Add support for that in xenconfig driver. When only one device is defined, keep using old syntax for backward compatibility. Adjust tests for changed options order. Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- src/xenconfig/xen_common.c | 66 - src/xenconfig/xen_xl.c | 127 + src/xenconfig/xen_xm.c | 72 ++ tests/xmconfigdata/test-fullvirt-usbmouse.cfg | 4 +- tests/xmconfigdata/test-fullvirt-usbtablet.cfg | 4 +- 5 files changed, 203 insertions(+), 70 deletions(-) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index a2a1474..079f77d 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -972,33 +972,6 @@ xenParseEmulatedDevices(virConfPtr conf, virDomainDefPtr def) if (str xenParseSxprSound(def, str) 0) return -1; - -if (xenConfigGetString(conf, usbdevice, str, NULL) 0) -return -1; - -if (str -(STREQ(str, tablet) || - STREQ(str, mouse) || - STREQ(str, keyboard))) { -virDomainInputDefPtr input; -if (VIR_ALLOC(input) 0) -return -1; - -input-bus = VIR_DOMAIN_INPUT_BUS_USB; -if (STREQ(str, mouse)) -input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE; -else if (STREQ(str, tablet)) -input-type = VIR_DOMAIN_INPUT_TYPE_TABLET; -else if (STREQ(str, keyboard)) -input-type = VIR_DOMAIN_INPUT_TYPE_KBD; -if (VIR_ALLOC_N(def-inputs, 1) 0) { -virDomainInputDefFree(input); -return -1; - -} -def-inputs[0] = input; -def-ninputs = 1; -} } return 0; @@ -1949,42 +1922,6 @@ xenFormatSound(virConfPtr conf, virDomainDefPtr def) } -static int -xenFormatInputDevs(virConfPtr conf, virDomainDefPtr def) -{ -size_t i; - -if (STREQ(def-os.type, hvm)) { -for (i = 0; i def-ninputs; i++) { -if (def-inputs[i]-bus == VIR_DOMAIN_INPUT_BUS_USB) { -if (xenConfigSetInt(conf, usb, 1) 0) -return -1; - -switch (def-inputs[i]-type) { -case VIR_DOMAIN_INPUT_TYPE_MOUSE: -if (xenConfigSetString(conf, usbdevice, mouse) 0) -return -1; - -break; -case VIR_DOMAIN_INPUT_TYPE_TABLET: -if (xenConfigSetString(conf, usbdevice, tablet) 0) -return -1; - -break; -case VIR_DOMAIN_INPUT_TYPE_KBD: -if (xenConfigSetString(conf, usbdevice, keyboard) 0) -return -1; - -break; -} -break; -} -} -} - -return 0; -} - static int xenFormatVif(virConfPtr conf, @@ -2059,9 +1996,6 @@ xenFormatConfigCommon(virConfPtr conf, if (xenFormatEmulator(conf, def) 0) return -1; -if (xenFormatInputDevs(conf, def) 0) -return -1; - if (xenFormatVfb(conf, def, xendConfigVersion) 0) return -1; diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 95ef5f4..f127ebe 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -289,6 +289,59 @@ xenParseXLDisk(virConfPtr conf, virDomainDefPtr def) goto cleanup; } +static int +xenParseXLInputDevs(virConfPtr conf, virDomainDefPtr def) +{ +const char *str; +virConfValuePtr val; + +if (STREQ(def-os.type, hvm)) { +val = virConfGetValue(conf, usbdevice); +/* usbdevice can be defined as either a single string or a list */ +if (val val-type == VIR_CONF_LIST) { +#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST +val = val-list; +#else +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(multiple USB devices not supported)); +return -1; +#endif +} +/* otherwise val-next is NULL, so can be handled by the same code */ +while (val) { +if (val-type != VIR_CONF_STRING) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(config value %s was malformed), + usbdevice); +return -1; +} +str = val-str; + +if (str +(STREQ(str, tablet) || + STREQ(str, mouse) || +
Re: [libvirt] [PATCH 0/3] qemu: fix broken block job handling
On 03/13/2015 10:25 AM, Peter Krempa wrote: Block job handling violates our usage of domain jobs and changes disk source definition behind our back. Peter Krempa (3): qemu: process: Export qemuProcessFindDomainDiskByAlias qemu: event: Don't fiddle with disk backing trees without a job qemu: Disallow concurrent block jobs on a single disk On description alone, this should help solve the crash that Shanzhi observed: https://www.redhat.com/archives/libvir-list/2015-March/msg00656.html src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 23 +++ src/qemu/qemu_domain.h | 4 ++ src/qemu/qemu_driver.c | 170 +++- src/qemu/qemu_process.c | 131 +++-- src/qemu/qemu_process.h | 3 + 6 files changed, 211 insertions(+), 121 deletions(-) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/9] libxl: prevent attaching multiple netdevs with the same MAC
Jim Fehlig wrote: Marek Marczykowski-Górecki wrote: It will not be possible to detach such device later. Also improve logging in such cases. Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- src/libxl/libxl_driver.c | 15 +++ 1 file changed, 15 insertions(+) Changes in v4: - use virDomainHasNet instead of virDomainNetFindIdx diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ce3a99b..1313d2e 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2787,6 +2787,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, int actualType; libxl_device_nic nic; int ret = -1; +char mac[VIR_MAC_STRING_BUFLEN]; /* preallocate new slot for device */ if (VIR_REALLOC_N(vm-def-nets, vm-def-nnets + 1) 0) @@ -2801,6 +2802,13 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, actualType = virDomainNetGetActualType(net); +if (virDomainHasNet(vm-def, net)) { +virReportError(VIR_ERR_OPERATION_FAILED, +_(device matching mac address %s already exists), + virMacAddrFormat(net-mac, mac)); I think the error should be VIR_ERR_INVALID_ARG, like you've done in the last hunk below. I'll change that, and use the same text in both error reports, before pushing. Otherwise, ACK. With the exception of 4 and 5, the patches in this series are unrelated. I have a question about 6, but in the meantime it seemed like a good point to push the first 5. Done now. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: event: Don't fiddle with disk backing trees without a job
On 03/13/2015 10:25 AM, Peter Krempa wrote: Surprisingly we did not grab a VM job when a block job finished and we'd happily rewrite the backing chain data. This made it possible to crash libvirt when queueing two backing chains tightly and other badness. My fault for violating the rule of 'no VM modifications without a job'. Thanks for finding and fixing this. To fix it, add yet another handler to the helper thread that handles monitor events that require a job. --- src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 142 src/qemu/qemu_process.c | 129 --- 3 files changed, 168 insertions(+), 105 deletions(-) +processBlockJobEvent(virQEMUDriverPtr driver, + virDomainObjPtr vm, + char *diskAlias, + int type, + int status) +{ +virObjectEventPtr event = NULL; +virObjectEventPtr event2 = NULL; +const char *path; +virDomainDiskDefPtr disk; +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +virDomainDiskDefPtr persistDisk = NULL; +bool save = false; + +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +VIR_DEBUG(Domain is not running); +goto endjob; +} Hmm. I suspect we could hit the situation where the pivot after active block commit or block copy happens just before the guest quits, and thus where we fail to update the XML to record that the pivot happened and instead leave the XML in its old state. Probably not the end of the world (at that point, the amount of work done by the guest after pivot is not much, so restarting the guest from the unpivoted disk is hopefully not inconsistent); except that a guest shutting down might make the small modification of marking a file system clean that turns out to have a large impact on how the guest next starts if it starts from the wrong disk. I don't know if we can do any better, and at a minimum this is a strict improvement over what we had before, so I'm not going to reject the patch because of it, but it is food for thought. [Actually, we probably need the notion of persistent bitmaps, which looks like it might hit qemu 2.4, before we can guarantee that we KNOW when a pivot job completed or failed, and thus always update the XML correctly, based on the state of the persistent bitmap file] virObjectLock(vm); -disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); -if (disk) { The old code was locking the VM, but modifying without a job. +processEvent-eventType = QEMU_PROCESS_EVENT_BLOCK_JOB; +if (VIR_STRDUP(data, diskAlias) 0) +goto error; +processEvent-data = data; +processEvent-vm = vm; +processEvent-action = type; +processEvent-status = status; -case VIR_DOMAIN_BLOCK_JOB_LAST: -break; -} +virObjectRef(vm); +if (virThreadPoolSendJob(driver-workerPool, 0, processEvent) 0) { +ignore_value(virObjectUnref(vm)); +goto error; The new code is now throwing things over the fence to a helper thread, so that blocking while waiting for the job is no longer holding up monitor interactions. Looks correct to me. ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] netdev: silence valgrind warning about ioctl use
Valgrind complained: ==3770== Syscall param ioctl(SIOCETHTOOL) points to uninitialised byte(s) ==3770==at 0x919D407: ioctl (syscall-template.S:81) ==3770==by 0x530FE7E: rpl_ioctl (ioctl.c:42) ==3770==by 0x50CB433: virNetDevFeatureAvailable (virnetdev.c:2764) ==3770==by 0x50CB6A7: virNetDevGetFeatures (virnetdev.c:2830) ==3770==by 0x1F0E5347: udevProcessNetworkInterface (node_device_udev.c:722) ==3770==by 0x1F0E689F: udevGetDeviceDetails (node_device_udev.c:1300) ==3770==by 0x1F0E6E06: udevAddOneDevice (node_device_udev.c:1422) ==3770==by 0x1F0E6FB8: udevProcessDeviceListEntry (node_device_udev.c:1464) ==3770==by 0x1F0E70CF: udevEnumerateDevices (node_device_udev.c:1494) ==3770==by 0x1F0E7BB4: nodeStateInitialize (node_device_udev.c:1806) ==3770==by 0x51B4303: virStateInitialize (libvirt.c:777) ==3770==by 0x11DEE7: daemonRunStateInit (libvirtd.c:906) ==3770== Address 0x228e38d4 is on thread 12's stack ==3770== in frame #2, created by virNetDevFeatureAvailable (virnetdev.c:2750) * src/util/virnetdev.c (virNetDevFeatureAvailable): Initialize all bytes of ifr. Signed-off-by: Eric Blake ebl...@redhat.com --- src/util/virnetdev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 093c99c..54d866e 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2758,6 +2758,7 @@ virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) goto cleanup; } +memset(ifr, 0, sizeof(ifr)); strcpy(ifr.ifr_name, ifname); ifr.ifr_data = (void*) cmd; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] target-i386: Haswell-noTSX and Broadwell-noTSX CPU models
On Fri, Mar 13, 2015 at 08:25:19PM +0100, Andreas Färber wrote: Am 13.03.2015 um 20:09 schrieb Eduardo Habkost: With the Intel microcode update that removed HLE and RTM, there will be different kinds of Haswell and Broadwell CPUs out there: some that still have the HLE and RTM features, and some that don't have the HLE and RTM features. On both cases people may be willing to use the pc-*-2.3 machine-types. So instead of making the CPU model results confusing by making it depend on the machine-type, keep HLE and RTM on the existing Haswell and Broadwell CPU models, and introduce Haswell-noTSX and Broadwell-noTSX CPU models later, for people who have CPUs that don't have TSX feature available. Eduardo Habkost (2): Revert target-i386: Disable HLE and RTM on Haswell Broadwell target-i386: Haswell-noTSX and Broadwell-noTSX No objections from a generic CPU point of view. Only thing that comes to mind is whether it might make sense to hierarchically make Broadwell the parent type of Broadwell-noTSX, to avoid duplication. But then again we already have a lot of it. ;) The builtin_x86_defs[] table has no way to represent inheritance, currently. Maybe one day when we move everything inside class_init functions. :) -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] libxl: Stubdom emulator type
Marek Marczykowski-Górecki wrote: Xen have feature of having device model in separate domain (called stub domain). Add stubdomain element to allow selecting such configuration. Emulator path is still used for qemu running in dom0 (if any). Libxl currently do not allow to select stubdomain path. Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- docs/formatdomain.html.in | 13 + docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c| 14 ++ src/conf/domain_conf.h| 3 ++- src/libxl/libxl_conf.c| 8 5 files changed, 47 insertions(+), 1 deletion(-) Changes in v4: - change emulator type='stubdom' to separate stubdomain element diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fb0a0d1..054f48f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1670,6 +1670,19 @@ /dd /dl +dl + dtcodestubdomain/code/dt + dd +span class=sinceSince 1.2.13/span, the optional codestubdomain/code + element contains codeenabled/code yes/no/default attribute. If enabled + Emulator will be launched in stub domain (Xen only). Currently there + is no way to provide path to that emulator. Note that in most cases + this emulator will be in addition to one in dom0. Why does a stubdomain need an emulator (running in dom0) to serve as a backend? Wouldn't it be better to give the stubdomain direct access to the resources it needs? Otherwise, It seems to defeat stubdomain's purpose of moving the emulator execution out of dom0. At least the Xen wiki describes this as primary motivation for studomains http://wiki.xen.org/wiki/Device_Model_Stub_Domains + codeemulator/code element described above still controls the path + to the dom0 instance. + /dd +/dl + I think it would be good to add an example above the description. E.g. along the lines of Daniel's previous example devices emulator/usr/lib/xen/bin/qemu-system-i386/emulator stubdomain enabled=yes|no/ /devices Regards, Jim h4a name=elementsDisksHard drives, floppy disks, CDROMs/a/h4 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4e4fe9f..539db39 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2567,6 +2567,13 @@ ref name=absFilePath/ /element /define + define name=stubdomain +element name=stubdomain + attribute name=enabled +ref name=virYesNo/ + /attribute +/element + /define !-- A graphic description, currently in Xen only 2 types are supported: - sdl with optional display, xauth and fullscreen @@ -4007,6 +4014,9 @@ optional ref name=emulator/ /optional +optional + ref name=stubdomain/ +/optional zeroOrMore choice ref name=disk/ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c430e03..e327e55 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14107,6 +14107,9 @@ virDomainDefParseXML(xmlDocPtr xml, } def-emulator = virXPathString(string(./devices/emulator[1]), ctxt); +tmp = virXPathString(string(./devices/stubdomain/@enabled), ctxt); +if (tmp) +def-stubdomain = virTristateBoolTypeFromString(tmp); /* analysis of the disk devices */ if ((n = virXPathNodeSet(./devices/disk, ctxt, nodes)) 0) @@ -16064,6 +16067,14 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; } +if (src-stubdomain != dst-stubdomain) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, +_(Target domain stubdomain '%s' does not match source '%s'), +virTristateBoolTypeToString(dst-stubdomain), +virTristateBoolTypeToString(src-stubdomain)); +goto error; +} + if (!virDomainDefFeaturesCheckABIStability(src, dst)) goto error; @@ -20303,6 +20314,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferEscapeString(buf, emulator%s/emulator\n, def-emulator); +if (def-stubdomain) +virBufferAsprintf(buf, stubdomain enabled='%s'/\n, + virTristateBoolTypeToString(def-stubdomain)); for (n = 0; n def-ndisks; n++) if (virDomainDiskDefFormat(buf, def-disks[n], flags) 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 36c8131..446f4f0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2108,9 +2108,10 @@ struct _virDomainDef { virDomainOSDef os; char *emulator; -/* These three options are of type virTristateSwitch, +/* These four options are of type virTristateSwitch, * except
Re: [libvirt] [PATCH 3/3] qemu: Disallow concurrent block jobs on a single disk
On 03/13/2015 10:25 AM, Peter Krempa wrote: While qemu may be prepared to do this libvirt is not. Forbid the block ops until we fix our code. --- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 23 +++ src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 28 +--- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ea463cb..6f2df46 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -664,6 +664,7 @@ struct _virDomainDiskDef { int tray_status; /* enum virDomainDiskTray */ int removable; /* enum virTristateSwitch */ +bool blockjob; Might be worth a FIXME comment acknowledging that a bool will be insufficient when we update our code to support parallel jobs. + +bool +qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk) +{ +if (disk-mirror) { +virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, + _(disk '%s' already in active block job), + disk-dst); + +return true; +} + +if (disk-blockjob) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _(disk '%s' already in active block job), + disk-dst); +return true; +} Shorter as 'if (disk-mirror || disk-blockjob)', up to you if you want to compress the common code. ACK. Looks like you have correctly locked out blockpull, blockcopy, and blockcommit as the three jobs that are all mutually exclusive according to our current abilities. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] network: avoid memory leak of dnsmasq capabilities
Valgrind detected a leak: ==17820== 102 (56 direct, 46 indirect) bytes in 1 blocks are definitely lost in loss record 479 of 646 ==17820==at 0x4A08946: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==17820==by 0x508521A: virAllocVar (viralloc.c:560) ==17820==by 0x50D9FCA: virObjectNew (virobject.c:193) ==17820==by 0x50A4FD9: dnsmasqCapsNewEmpty (virdnsmasq.c:784) ==17820==by 0x50A514E: dnsmasqCapsNewFromBinary (virdnsmasq.c:830) ==17820==by 0x1B508287: networkStateInitialize (bridge_driver.c:666) It looks like commit 172acef introduced the problem, because networkGetDnsmasqCaps() increments the reference count but an early exit never does a matching decrement. * src/network/bridge_driver.c (networkStateCleanup): Plug leak. Signed-off-by: Eric Blake ebl...@redhat.com --- src/network/bridge_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 13e1717..a007388 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -391,8 +391,8 @@ networkUpdateState(virNetworkObjPtr obj, virObjectLock(obj); if (!virNetworkObjIsActive(obj)) { -virObjectUnlock(obj); -return 0; +ret = 0; +goto cleanup; } switch (obj-def-forward.type) { -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] netdev: silence valgrind warning about ioctl use
On 03/13/2015 04:38 PM, Eric Blake wrote: Valgrind complained: ==3770== Syscall param ioctl(SIOCETHTOOL) points to uninitialised byte(s) ==3770==at 0x919D407: ioctl (syscall-template.S:81) ==3770==by 0x530FE7E: rpl_ioctl (ioctl.c:42) ==3770==by 0x50CB433: virNetDevFeatureAvailable (virnetdev.c:2764) ==3770==by 0x50CB6A7: virNetDevGetFeatures (virnetdev.c:2830) ==3770==by 0x1F0E5347: udevProcessNetworkInterface (node_device_udev.c:722) ==3770==by 0x1F0E689F: udevGetDeviceDetails (node_device_udev.c:1300) ==3770==by 0x1F0E6E06: udevAddOneDevice (node_device_udev.c:1422) ==3770==by 0x1F0E6FB8: udevProcessDeviceListEntry (node_device_udev.c:1464) ==3770==by 0x1F0E70CF: udevEnumerateDevices (node_device_udev.c:1494) ==3770==by 0x1F0E7BB4: nodeStateInitialize (node_device_udev.c:1806) ==3770==by 0x51B4303: virStateInitialize (libvirt.c:777) ==3770==by 0x11DEE7: daemonRunStateInit (libvirtd.c:906) ==3770== Address 0x228e38d4 is on thread 12's stack ==3770== in frame #2, created by virNetDevFeatureAvailable (virnetdev.c:2750) * src/util/virnetdev.c (virNetDevFeatureAvailable): Initialize all bytes of ifr. Signed-off-by: Eric Blake ebl...@redhat.com --- src/util/virnetdev.c | 1 + 1 file changed, 1 insertion(+) ACK John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] util: don't fail if no PortData is found while getting migrateData
On 03/04/2015 09:01 PM, zhang bo wrote: Introduced by f6a2f97e Problem Description: After multiple times of migrating a domain, which has an ovs interface with no portData set, with non-shared disk, nbd ports got overflowed. The steps to reproduce the problem: 1 define and start a domain with its network configured as: interface type='bridge' source bridge='br0'/ virtualport type='openvswitch' /virtualport model type='virtio'/ driver name='vhost' queues='4'/ /interface 2 do not set the network's portData. 3 migrate(ToURI2) it with flag 91(1011011), which means: VIR_MIGRATE_LIVE VIR_MIGRATE_PEER2PEER VIR_MIGRATE_PERSIST_DEST VIR_MIGRATE_UNDEFINE_SOURCE VIR_MIGRATE_NON_SHARED_DISK 4 migrate success, but we got an error log in libvirtd.log: error : virCommandWait:2423 : internal error: Child process (ovs-vsctl --timeout=5 get Interface vnet1 external_ids:PortData) unexpected exit status 1: ovs-vsctl: no key PortData in Interface record vnet1 column external_ids 5 migrate it back, migrate it , migrate it back, ... 6 nbd port got overflowed. The reasons for the problem is : 1 virNetDevOpenvswitchGetMigrateData() takes it as wrong if no portData is available for the ovs interface of a domain. (We think it's not appropriate, as portData is just OPTIONAL) 2 in func qemuMigrationBakeCookie(), it fails in qemuMigrationCookieAddNetwork(), and returns with -1. qemuMigrationCookieAddNBD() is not called thereafter, and mig-nbd is still NULL. 3 However, qemuMigrationRun() just *WARN* if qemuMigrationBakeCookie() fails, migration still successes. cookie is NULL, it's not baked on the src side. 4 On the destination side, it would alloc a port first and then free the nbd port in COOKIE. But the cookie is NULL due to qemuMigrationCookieAddNetwork() failure at src side. thus the nbd port is not freed. In this patch, we add --if-exists option to make ovs-vsctl not raise error if there's no portData available. Further more, because portData may be NULL in the cookie at the dest side, check it before setting portData. Signed-off-by: Zhou Yimin zhouyi...@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- V1 here: https://www.redhat.com/archives/libvir-list/2015-February/msg00388.html V2: Add --if-exists option to ovs-vsctl cmd, making ovs-vsctl not raise error if there's no portData available. Suggested by Martin. We Tested the patch, it works. link: https://www.redhat.com/archives/libvir-list/2015-March/msg00104.html V3: move V1/V2/V3 description from above the patch to here. It did not show the detailed description above after apply PATCH v2. --- src/util/virnetdevopenvswitch.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) Looks like this is a stray... Seems to me you followed Martin's advice from his 2nd/3rd pass on your v1... Had some issues attempting to 'git am' the patch, but eventually found that the last diff stanza was off by a line as if someone edited something before sending... If I change: @@ -241,6 +244,12 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) to @@ -241,6 +244,11 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) Then the patch applied... In any case, ACK and I've pushed. John diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index e5c87bb..722d0dd 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -30,9 +30,12 @@ #include virerror.h #include virmacaddr.h #include virstring.h +#include virlog.h #define VIR_FROM_THIS VIR_FROM_NONE +VIR_LOG_INIT(util.netdevopenvswitch); + /** * virNetDevOpenvswitchAddPort: * @brname: the bridge name @@ -206,7 +209,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) virCommandPtr cmd = NULL; int ret = -1; -cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, get, Interface, +cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, --if-exists, get, Interface, ifname, external_ids:PortData, NULL); virCommandSetOutputBuffer(cmd, migrate); @@ -241,6 +244,12 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) virCommandPtr cmd = NULL; int ret = -1; +if (!migrate) { +VIR_DEBUG(No OVS port data for interface %s, ifname); +return 0; +} + cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, set, Interface, ifname, NULL); virCommandAddArgFormat(cmd, external_ids:PortData=%s, migrate); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/12] Replace virDomainVcpuPinAdd with virDomainPinAdd
Make common between Vcpu and IOThreads Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 42 -- src/conf/domain_conf.h | 10 +- src/libvirt_private.syms | 2 +- src/libxl/libxl_driver.c | 10 +- src/qemu/qemu_driver.c | 16 src/xen/xend_internal.c | 10 +- 6 files changed, 44 insertions(+), 46 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 75d75bc..4a9d083 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16711,47 +16711,45 @@ virDomainPinFindByVcpu(virDomainPinDefPtr *def, } int -virDomainVcpuPinAdd(virDomainPinDefPtr **vcpupin_list, -size_t *nvcpupin, -unsigned char *cpumap, -int maplen, -int vcpu) +virDomainPinAdd(virDomainPinDefPtr **pindef_list, +size_t *npin, +unsigned char *cpumap, +int maplen, +int id) { -virDomainPinDefPtr vcpupin = NULL; +virDomainPinDefPtr pindef = NULL; -if (!vcpupin_list) +if (!pindef_list) return -1; -vcpupin = virDomainPinFindByVcpu(*vcpupin_list, - *nvcpupin, - vcpu); -if (vcpupin) { -vcpupin-id = vcpu; -virBitmapFree(vcpupin-cpumask); -vcpupin-cpumask = virBitmapNewData(cpumap, maplen); -if (!vcpupin-cpumask) +pindef = virDomainPinFindByVcpu(*pindef_list, *npin, id); +if (pindef) { +pindef-id = id; +virBitmapFree(pindef-cpumask); +pindef-cpumask = virBitmapNewData(cpumap, maplen); +if (!pindef-cpumask) return -1; return 0; } -/* No existing vcpupin matches vcpu, adding a new one */ +/* No existing pindef matches id, adding a new one */ -if (VIR_ALLOC(vcpupin) 0) +if (VIR_ALLOC(pindef) 0) goto error; -vcpupin-id = vcpu; -vcpupin-cpumask = virBitmapNewData(cpumap, maplen); -if (!vcpupin-cpumask) +pindef-id = id; +pindef-cpumask = virBitmapNewData(cpumap, maplen); +if (!pindef-cpumask) goto error; -if (VIR_APPEND_ELEMENT(*vcpupin_list, *nvcpupin, vcpupin) 0) +if (VIR_APPEND_ELEMENT(*pindef_list, *npin, pindef) 0) goto error; return 0; error: -virDomainPinDefFree(vcpupin); +virDomainPinDefFree(pindef); return -1; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fe61b9f..10fdbb0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2549,11 +2549,11 @@ int virDomainDefCompatibleDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev, virDomainDeviceAction action); -int virDomainVcpuPinAdd(virDomainPinDefPtr **vcpupin_list, -size_t *nvcpupin, -unsigned char *cpumap, -int maplen, -int vcpu); +int virDomainPinAdd(virDomainPinDefPtr **pindef_list, +size_t *npin, +unsigned char *cpumap, +int maplen, +int id); void virDomainVcpuPinDel(virDomainDefPtr def, int vcpu); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 540936f..54c931e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -381,6 +381,7 @@ virDomainObjTaint; virDomainParseMemory; virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; +virDomainPinAdd; virDomainPinDefArrayFree; virDomainPinDefCopy; virDomainPinDefFree; @@ -437,7 +438,6 @@ virDomainTPMBackendTypeToString; virDomainTPMDefFree; virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; -virDomainVcpuPinAdd; virDomainVcpuPinDel; virDomainVideoDefaultRAM; virDomainVideoDefaultType; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 97ef2c9..d1addf5 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1999,11 +1999,11 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, goto endjob; targetDef-cputune.nvcpupin = 0; } -if (virDomainVcpuPinAdd(targetDef-cputune.vcpupin, -targetDef-cputune.nvcpupin, -cpumap, -maplen, -vcpu) 0) { +if (virDomainPinAdd(targetDef-cputune.vcpupin, +targetDef-cputune.nvcpupin, +cpumap, +maplen, +vcpu) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(failed to update or add vcpupin xml)); goto endjob; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5d867c3..3e40724 100644 --- a/src/qemu/qemu_driver.c +++
[libvirt] [PATCH 01/12] qemu: Fix possible memory leak in qemuDomainPinVcpuFlags
During his review of the iothreads pin setting code, Pavel noted that there was a potential memory leak with respect to how the newVcpuPin is handled and the goto endjob's in failure paths which would not free the memory. For reference, See: http://www.redhat.com/archives/libvir-list/2015-March/msg00415.html Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_driver.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3263ac..dea0788 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4978,10 +4978,10 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, newVcpuPinNum = 0; } -if (virDomainVcpuPinAdd(newVcpuPin, newVcpuPinNum, cpumap, maplen, vcpu) 0) { +if (virDomainVcpuPinAdd(newVcpuPin, newVcpuPinNum, +cpumap, maplen, vcpu) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(failed to update vcpupin)); -virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); goto endjob; } @@ -4989,7 +4989,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, if (virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { if (virCgroupNewVcpu(priv-cgroup, vcpu, false, cgroup_vcpu) 0) goto endjob; -if (qemuSetupCgroupVcpuPin(cgroup_vcpu, newVcpuPin, newVcpuPinNum, vcpu) 0) { +if (qemuSetupCgroupVcpuPin(cgroup_vcpu, newVcpuPin, newVcpuPinNum, + vcpu) 0) { virReportError(VIR_ERR_OPERATION_INVALID, _(failed to set cpuset.cpus in cgroup for vcpu %d), vcpu); @@ -5008,16 +5009,14 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, virDomainVcpuPinDel(vm-def, vcpu); } else { if (vm-def-cputune.vcpupin) -virDomainVcpuPinDefArrayFree(vm-def-cputune.vcpupin, vm-def-cputune.nvcpupin); +virDomainVcpuPinDefArrayFree(vm-def-cputune.vcpupin, + vm-def-cputune.nvcpupin); vm-def-cputune.vcpupin = newVcpuPin; vm-def-cputune.nvcpupin = newVcpuPinNum; newVcpuPin = NULL; } -if (newVcpuPin) -virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); - if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0) goto endjob; @@ -5066,6 +5065,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: +if (newVcpuPin) +virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); if (cgroup_vcpu) virCgroupFree(cgroup_vcpu); qemuDomObjEndAPI(vm); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/12] Convert virDomainVcpuPinDefArrayFree to virDomainPinDefArrayFree
Make common between Vcpu and IOThreads Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 10 +- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 16 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db04855..e5a8011 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2046,15 +2046,15 @@ virDomainPinDefFree(virDomainPinDefPtr def) } void -virDomainVcpuPinDefArrayFree(virDomainPinDefPtr *def, - int nvcpupin) +virDomainPinDefArrayFree(virDomainPinDefPtr *def, + int npin) { size_t i; if (!def) return; -for (i = 0; i nvcpupin; i++) +for (i = 0; i npin; i++) virDomainPinDefFree(def[i]); VIR_FREE(def); @@ -2229,11 +2229,11 @@ void virDomainDefFree(virDomainDefPtr def) virCPUDefFree(def-cpu); -virDomainVcpuPinDefArrayFree(def-cputune.vcpupin, def-cputune.nvcpupin); +virDomainPinDefArrayFree(def-cputune.vcpupin, def-cputune.nvcpupin); virDomainPinDefFree(def-cputune.emulatorpin); -virDomainVcpuPinDefArrayFree(def-cputune.iothreadspin, +virDomainPinDefArrayFree(def-cputune.iothreadspin, def-cputune.niothreadspin); for (i = 0; i def-cputune.nvcpusched; i++) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3f04832..a126668 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1904,7 +1904,7 @@ struct _virDomainPinDef { }; void virDomainPinDefFree(virDomainPinDefPtr def); -void virDomainVcpuPinDefArrayFree(virDomainPinDefPtr *def, int nvcpupin); +void virDomainPinDefArrayFree(virDomainPinDefPtr *def, int npin); virDomainPinDefPtr *virDomainVcpuPinDefCopy(virDomainPinDefPtr *src, int nvcpupin); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index adcb456..8796b31 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -381,6 +381,7 @@ virDomainObjTaint; virDomainParseMemory; virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; +virDomainPinDefArrayFree; virDomainPinDefFree; virDomainPMSuspendedReasonTypeFromString; virDomainPMSuspendedReasonTypeToString; @@ -434,7 +435,6 @@ virDomainTPMDefFree; virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; virDomainVcpuPinAdd; -virDomainVcpuPinDefArrayFree; virDomainVcpuPinDefCopy; virDomainVcpuPinDel; virDomainVcpuPinFindByVcpu; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fed0c0d..b3eacf8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5009,8 +5009,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, virDomainVcpuPinDel(vm-def, vcpu); } else { if (vm-def-cputune.vcpupin) -virDomainVcpuPinDefArrayFree(vm-def-cputune.vcpupin, - vm-def-cputune.nvcpupin); +virDomainPinDefArrayFree(vm-def-cputune.vcpupin, + vm-def-cputune.nvcpupin); vm-def-cputune.vcpupin = newVcpuPin; vm-def-cputune.nvcpupin = newVcpuPinNum; @@ -5066,7 +5066,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, cleanup: if (newVcpuPin) -virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); +virDomainPinDefArrayFree(newVcpuPin, newVcpuPinNum); if (cgroup_vcpu) virCgroupFree(cgroup_vcpu); qemuDomObjEndAPI(vm); @@ -5258,7 +5258,7 @@ qemuDomainPinEmulator(virDomainPtr dom, if (virDomainVcpuPinAdd(newVcpuPin, newVcpuPinNum, cpumap, maplen, -1) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(failed to update vcpupin)); -virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); +virDomainPinDefArrayFree(newVcpuPin, newVcpuPinNum); goto endjob; } @@ -5299,7 +5299,7 @@ qemuDomainPinEmulator(virDomainPtr dom, } if (newVcpuPin) -virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); +virDomainPinDefArrayFree(newVcpuPin, newVcpuPinNum); } else { virReportError(VIR_ERR_OPERATION_INVALID, %s, _(cpu affinity is not supported)); @@ -5886,8 +5886,8 @@ qemuDomainPinIOThread(virDomainPtr dom, } if (vm-def-cputune.iothreadspin) -virDomainVcpuPinDefArrayFree(vm-def-cputune.iothreadspin, - vm-def-cputune.niothreadspin); +virDomainPinDefArrayFree(vm-def-cputune.iothreadspin, + vm-def-cputune.niothreadspin); vm-def-cputune.iothreadspin = newIOThreadsPin; vm-def-cputune.niothreadspin =
[libvirt] [PATCH 06/12] Convert virDomainVcpuPinDefCopy into virDomainPinDefCopy
Make common between Vcpu and IOThreads Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 8 src/conf/domain_conf.h | 4 ++-- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 9 - 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e5a8011..63e7c77 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2004,15 +2004,15 @@ virDomainClockDefClear(virDomainClockDefPtr def) } virDomainPinDefPtr * -virDomainVcpuPinDefCopy(virDomainPinDefPtr *src, int nvcpupin) +virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) { size_t i; virDomainPinDefPtr *ret = NULL; -if (VIR_ALLOC_N(ret, nvcpupin) 0) +if (VIR_ALLOC_N(ret, npin) 0) goto error; -for (i = 0; i nvcpupin; i++) { +for (i = 0; i npin; i++) { if (VIR_ALLOC(ret[i]) 0) goto error; ret[i]-id = src[i]-id; @@ -2024,7 +2024,7 @@ virDomainVcpuPinDefCopy(virDomainPinDefPtr *src, int nvcpupin) error: if (ret) { -for (i = 0; i nvcpupin; i++) { +for (i = 0; i npin; i++) { if (ret[i]) { virBitmapFree(ret[i]-cpumask); VIR_FREE(ret[i]); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a126668..5fc4074 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1906,8 +1906,8 @@ struct _virDomainPinDef { void virDomainPinDefFree(virDomainPinDefPtr def); void virDomainPinDefArrayFree(virDomainPinDefPtr *def, int npin); -virDomainPinDefPtr *virDomainVcpuPinDefCopy(virDomainPinDefPtr *src, -int nvcpupin); +virDomainPinDefPtr *virDomainPinDefCopy(virDomainPinDefPtr *src, +int npin); int virDomainVcpuPinIsDuplicate(virDomainPinDefPtr *def, int nvcpupin, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8796b31..33a50a9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -382,6 +382,7 @@ virDomainParseMemory; virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; virDomainPinDefArrayFree; +virDomainPinDefCopy; virDomainPinDefFree; virDomainPMSuspendedReasonTypeFromString; virDomainPMSuspendedReasonTypeToString; @@ -435,7 +436,6 @@ virDomainTPMDefFree; virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; virDomainVcpuPinAdd; -virDomainVcpuPinDefCopy; virDomainVcpuPinDel; virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3eacf8..0d6f367 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4966,8 +4966,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, } if (vm-def-cputune.vcpupin) { -newVcpuPin = virDomainVcpuPinDefCopy(vm-def-cputune.vcpupin, - vm-def-cputune.nvcpupin); +newVcpuPin = virDomainPinDefCopy(vm-def-cputune.vcpupin, + vm-def-cputune.nvcpupin); if (!newVcpuPin) goto endjob; @@ -5839,10 +5839,9 @@ qemuDomainPinIOThread(virDomainPtr dom, } if (vm-def-cputune.iothreadspin) { -/* The VcpuPinDefCopy works for IOThreads too */ newIOThreadsPin = -virDomainVcpuPinDefCopy(vm-def-cputune.iothreadspin, -vm-def-cputune.niothreadspin); +virDomainPinDefCopy(vm-def-cputune.iothreadspin, +vm-def-cputune.niothreadspin); if (!newIOThreadsPin) goto endjob; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/12] Convert virDomainPinDefPtr-vcpuid to virDomainPinDefPtr-id
Since we're not specifically a vcpu related structure anymore... Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 39 +++ src/conf/domain_conf.h | 2 +- src/libxl/libxl_domain.c | 2 +- src/libxl/libxl_driver.c | 2 +- src/qemu/qemu_cgroup.c | 8 src/qemu/qemu_driver.c | 2 +- 6 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e54dc1..04de04e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2015,7 +2015,7 @@ virDomainVcpuPinDefCopy(virDomainPinDefPtr *src, int nvcpupin) for (i = 0; i nvcpupin; i++) { if (VIR_ALLOC(ret[i]) 0) goto error; -ret[i]-vcpuid = src[i]-vcpuid; +ret[i]-id = src[i]-id; if ((ret[i]-cpumask = virBitmapNewCopy(src[i]-cpumask)) == NULL) goto error; } @@ -12736,7 +12736,7 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node, goto error; } -def-vcpuid = vcpuid; +def-id = vcpuid; } if (iothreads (tmp = virXPathString(string(./@iothread), ctxt))) { @@ -12763,8 +12763,7 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node, goto error; } -/* Rather than creating our own structure we are reusing the vCPU */ -def-vcpuid = iothreadid; +def-id = iothreadid; } if (!(tmp = virXMLPropString(node, cpuset))) { @@ -13472,14 +13471,14 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainVcpuPinIsDuplicate(def-cputune.vcpupin, def-cputune.nvcpupin, -vcpupin-vcpuid)) { +vcpupin-id)) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(duplicate vcpupin for same vcpu)); virDomainVcpuPinDefFree(vcpupin); goto error; } -if (vcpupin-vcpuid = def-vcpus) { +if (vcpupin-id = def-vcpus) { /* To avoid the regression when daemon loading * domain confs, we can't simply error out if * vcpupin nodes greater than current vcpus, @@ -13516,7 +13515,7 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } virBitmapCopy(vcpupin-cpumask, def-cpumask); -vcpupin-vcpuid = i; +vcpupin-id = i; def-cputune.vcpupin[def-cputune.nvcpupin++] = vcpupin; } } @@ -13564,7 +13563,7 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainVcpuPinIsDuplicate(def-cputune.iothreadspin, def-cputune.niothreadspin, -iothreadpin-vcpuid)) { +iothreadpin-id)) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(duplicate iothreadpin for same iothread)); virDomainVcpuPinDefFree(iothreadpin); @@ -16673,7 +16672,7 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) return 0; } -/* Check if vcpupin with same vcpuid already exists. +/* Check if vcpupin with same id already exists. * Return 1 if exists, 0 if not. */ int virDomainVcpuPinIsDuplicate(virDomainPinDefPtr *def, @@ -16686,7 +16685,7 @@ virDomainVcpuPinIsDuplicate(virDomainPinDefPtr *def, return 0; for (i = 0; i nvcpupin; i++) { -if (def[i]-vcpuid == vcpu) +if (def[i]-id == vcpu) return 1; } @@ -16704,7 +16703,7 @@ virDomainVcpuPinFindByVcpu(virDomainPinDefPtr *def, return NULL; for (i = 0; i nvcpupin; i++) { -if (def[i]-vcpuid == vcpu) +if (def[i]-id == vcpu) return def[i]; } @@ -16727,7 +16726,7 @@ virDomainVcpuPinAdd(virDomainPinDefPtr **vcpupin_list, *nvcpupin, vcpu); if (vcpupin) { -vcpupin-vcpuid = vcpu; +vcpupin-id = vcpu; virBitmapFree(vcpupin-cpumask); vcpupin-cpumask = virBitmapNewData(cpumap, maplen); if (!vcpupin-cpumask) @@ -16741,7 +16740,7 @@ virDomainVcpuPinAdd(virDomainPinDefPtr **vcpupin_list, if (VIR_ALLOC(vcpupin) 0) goto error; -vcpupin-vcpuid = vcpu; +vcpupin-id = vcpu; vcpupin-cpumask = virBitmapNewData(cpumap, maplen); if (!vcpupin-cpumask) goto error; @@ -16763,7 +16762,7 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) virDomainPinDefPtr *vcpupin_list = def-cputune.vcpupin; for (n = 0; n def-cputune.nvcpupin; n++) { -if (vcpupin_list[n]-vcpuid == vcpu) { +if (vcpupin_list[n]-id == vcpu) { virBitmapFree(vcpupin_list[n]-cpumask); VIR_FREE(vcpupin_list[n]); VIR_DELETE_ELEMENT(def-cputune.vcpupin, n, def-cputune.nvcpupin); @@ -16784,7 +16783,7 @@
[libvirt] [PATCH 02/12] Convert virDomainVcpuPinDefPtr to virDomainPinDefPtr
As pointed out by jtomko in his review of the IOThreads pinning code: http://www.redhat.com/archives/libvir-list/2015-March/msg00495.html there are some comments sprinkled in indicating IOThreads were using the same structure as the VcpuPin code... This is the first patch of a few that will change the virDomainVcpuPin* structures and code to just virDomainPin* - starting with the data structure naming... Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 42 -- src/conf/domain_conf.h | 33 - src/libxl/libxl_driver.c | 2 +- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_cgroup.h | 4 ++-- src/qemu/qemu_driver.c | 12 ++-- src/qemu/qemu_process.c | 4 ++-- 7 files changed, 49 insertions(+), 52 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3d05844..6e54dc1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2003,11 +2003,11 @@ virDomainClockDefClear(virDomainClockDefPtr def) VIR_FREE(def-timers); } -virDomainVcpuPinDefPtr * -virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin) +virDomainPinDefPtr * +virDomainVcpuPinDefCopy(virDomainPinDefPtr *src, int nvcpupin) { size_t i; -virDomainVcpuPinDefPtr *ret = NULL; +virDomainPinDefPtr *ret = NULL; if (VIR_ALLOC_N(ret, nvcpupin) 0) goto error; @@ -2037,7 +2037,7 @@ virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin) } void -virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr def) +virDomainVcpuPinDefFree(virDomainPinDefPtr def) { if (def) { virBitmapFree(def-cpumask); @@ -2046,7 +2046,7 @@ virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr def) } void -virDomainVcpuPinDefArrayFree(virDomainVcpuPinDefPtr *def, +virDomainVcpuPinDefArrayFree(virDomainPinDefPtr *def, int nvcpupin) { size_t i; @@ -12699,14 +12699,14 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, * A vcpuid of -1 is valid and only valid for emulatorpin. So callers * have to check the returned cpuid for validity. */ -static virDomainVcpuPinDefPtr +static virDomainPinDefPtr virDomainVcpuPinDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, int maxvcpus, bool emulator, bool iothreads) { -virDomainVcpuPinDefPtr def; +virDomainPinDefPtr def; xmlNodePtr oldnode = ctxt-node; int vcpuid = -1; unsigned int iothreadid; @@ -13463,7 +13463,7 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; for (i = 0; i n; i++) { -virDomainVcpuPinDefPtr vcpupin = NULL; +virDomainPinDefPtr vcpupin = NULL; vcpupin = virDomainVcpuPinDefParseXML(nodes[i], ctxt, def-maxvcpus, false, false); @@ -13506,7 +13506,7 @@ virDomainDefParseXML(xmlDocPtr xml, i)) continue; -virDomainVcpuPinDefPtr vcpupin = NULL; +virDomainPinDefPtr vcpupin = NULL; if (VIR_ALLOC(vcpupin) 0) goto error; @@ -13555,7 +13555,7 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; for (i = 0; i n; i++) { -virDomainVcpuPinDefPtr iothreadpin = NULL; +virDomainPinDefPtr iothreadpin = NULL; iothreadpin = virDomainVcpuPinDefParseXML(nodes[i], ctxt, def-iothreads, false, true); @@ -16676,7 +16676,7 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) /* Check if vcpupin with same vcpuid already exists. * Return 1 if exists, 0 if not. */ int -virDomainVcpuPinIsDuplicate(virDomainVcpuPinDefPtr *def, +virDomainVcpuPinIsDuplicate(virDomainPinDefPtr *def, int nvcpupin, int vcpu) { @@ -16693,8 +16693,8 @@ virDomainVcpuPinIsDuplicate(virDomainVcpuPinDefPtr *def, return 0; } -virDomainVcpuPinDefPtr -virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, +virDomainPinDefPtr +virDomainVcpuPinFindByVcpu(virDomainPinDefPtr *def, int nvcpupin, int vcpu) { @@ -16712,13 +16712,13 @@ virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, } int -virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, +virDomainVcpuPinAdd(virDomainPinDefPtr **vcpupin_list, size_t *nvcpupin, unsigned char *cpumap, int maplen, int vcpu) { -virDomainVcpuPinDefPtr vcpupin = NULL; +virDomainPinDefPtr vcpupin = NULL; if (!vcpupin_list) return -1; @@ -16760,7 +16760,7 @@ void virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) { int n; -virDomainVcpuPinDefPtr
[libvirt] [PATCH 07/12] Convert virDomainVcpuPinIsDuplicate into virDomainPinIsDuplicate
Make common between Vcpu and IOThreads Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 30 +++--- src/conf/domain_conf.h | 6 +++--- src/libvirt_private.syms | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 63e7c77..efc01bd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13469,9 +13469,9 @@ virDomainDefParseXML(xmlDocPtr xml, if (!vcpupin) goto error; -if (virDomainVcpuPinIsDuplicate(def-cputune.vcpupin, -def-cputune.nvcpupin, -vcpupin-id)) { +if (virDomainPinIsDuplicate(def-cputune.vcpupin, +def-cputune.nvcpupin, +vcpupin-id)) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(duplicate vcpupin for same vcpu)); virDomainPinDefFree(vcpupin); @@ -13500,9 +13500,9 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; for (i = 0; i def-vcpus; i++) { -if (virDomainVcpuPinIsDuplicate(def-cputune.vcpupin, -def-cputune.nvcpupin, -i)) +if (virDomainPinIsDuplicate(def-cputune.vcpupin, +def-cputune.nvcpupin, +i)) continue; virDomainPinDefPtr vcpupin = NULL; @@ -13561,9 +13561,9 @@ virDomainDefParseXML(xmlDocPtr xml, if (!iothreadpin) goto error; -if (virDomainVcpuPinIsDuplicate(def-cputune.iothreadspin, -def-cputune.niothreadspin, -iothreadpin-id)) { +if (virDomainPinIsDuplicate(def-cputune.iothreadspin, +def-cputune.niothreadspin, +iothreadpin-id)) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(duplicate iothreadpin for same iothread)); virDomainPinDefFree(iothreadpin); @@ -16675,17 +16675,17 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) /* Check if vcpupin with same id already exists. * Return 1 if exists, 0 if not. */ int -virDomainVcpuPinIsDuplicate(virDomainPinDefPtr *def, -int nvcpupin, -int vcpu) +virDomainPinIsDuplicate(virDomainPinDefPtr *def, +int npin, +int id) { size_t i; -if (!def || !nvcpupin) +if (!def || !npin) return 0; -for (i = 0; i nvcpupin; i++) { -if (def[i]-id == vcpu) +for (i = 0; i npin; i++) { +if (def[i]-id == id) return 1; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5fc4074..0fb8d01 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1909,9 +1909,9 @@ void virDomainPinDefArrayFree(virDomainPinDefPtr *def, int npin); virDomainPinDefPtr *virDomainPinDefCopy(virDomainPinDefPtr *src, int npin); -int virDomainVcpuPinIsDuplicate(virDomainPinDefPtr *def, -int nvcpupin, -int vcpu); +int virDomainPinIsDuplicate(virDomainPinDefPtr *def, +int npin, +int id); virDomainPinDefPtr virDomainVcpuPinFindByVcpu(virDomainPinDefPtr *def, int nvcpupin, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 33a50a9..365430f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -384,6 +384,7 @@ virDomainPausedReasonTypeToString; virDomainPinDefArrayFree; virDomainPinDefCopy; virDomainPinDefFree; +virDomainPinIsDuplicate; virDomainPMSuspendedReasonTypeFromString; virDomainPMSuspendedReasonTypeToString; virDomainRedirdevBusTypeFromString; @@ -438,7 +439,6 @@ virDomainTPMModelTypeToString; virDomainVcpuPinAdd; virDomainVcpuPinDel; virDomainVcpuPinFindByVcpu; -virDomainVcpuPinIsDuplicate; virDomainVideoDefaultRAM; virDomainVideoDefaultType; virDomainVideoDefFree; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/12] More cleanup from IOThreads changes
During the review process a few things were pointed at as perhaps needing some adjustments based on what was done for IOThreads. Specifically a memory leak in PinVcpuFlags since PinIOThreads was just a copy of the Vcpu code and secondarily since the IOThreads code reused the virDomainVcpuPin* data structures and API's, those should change names to be more generic. John Ferlan (12): qemu: Fix possible memory leak in qemuDomainPinVcpuFlags Convert virDomainVcpuPinDefPtr to virDomainPinDefPtr Convert virDomainPinDefPtr-vcpuid to virDomainPinDefPtr-id Convert virDomainVcpuPinDefFree to virDomainPinDefFree Convert virDomainVcpuPinDefArrayFree to virDomainPinDefArrayFree Convert virDomainVcpuPinDefCopy into virDomainPinDefCopy Convert virDomainVcpuPinIsDuplicate into virDomainPinIsDuplicate Convert virDomainVcpuPinFindByVcpu into virDomainPinFindByVcpu Replace virDomainVcpuPinAdd with virDomainPinAdd Replace virDomainIOThreadsPinAdd with virDomainPinAdd Replace virDomainVcpuPinDel with virDomainPinDel Remove virDomainIOThreadsPinDel src/conf/domain_conf.c | 234 +-- src/conf/domain_conf.h | 58 +--- src/libvirt_private.syms | 16 ++-- src/libxl/libxl_domain.c | 2 +- src/libxl/libxl_driver.c | 18 ++-- src/qemu/qemu_cgroup.c | 12 +-- src/qemu/qemu_cgroup.h | 4 +- src/qemu/qemu_driver.c | 104 +++-- src/qemu/qemu_process.c | 16 ++-- src/xen/xend_internal.c | 10 +- 10 files changed, 204 insertions(+), 270 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/12] Remove virDomainIOThreadsPinDel
This one is no longer necessary Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 17 - src/conf/domain_conf.h | 3 --- src/libvirt_private.syms | 1 - 3 files changed, 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 24c0d8e..cfb59ca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16815,23 +16815,6 @@ virDomainEmulatorPinDel(virDomainDefPtr def) return 0; } -void -virDomainIOThreadsPinDel(virDomainDefPtr def, - unsigned int iothread_id) -{ -size_t i; -virDomainPinDefPtr *iothreadspin_list = def-cputune.iothreadspin; - -for (i = 0; i def-cputune.niothreadspin; i++) { -if (iothreadspin_list[i]-id == iothread_id) { -virBitmapFree(iothreadspin_list[i]-cpumask); -VIR_DELETE_ELEMENT(def-cputune.iothreadspin, i, - def-cputune.niothreadspin); -return; -} -} -} - static int virDomainEventActionDefFormat(virBufferPtr buf, int type, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c10e080..d88aca3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2565,9 +2565,6 @@ int virDomainEmulatorPinAdd(virDomainDefPtr def, int virDomainEmulatorPinDel(virDomainDefPtr def); -void virDomainIOThreadsPinDel(virDomainDefPtr def, - unsigned int iothread_id); - void virDomainRNGDefFree(virDomainRNGDefPtr def); bool virDomainDiskDefDstDuplicates(virDomainDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c453fb8..490c406 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -309,7 +309,6 @@ virDomainHubTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; virDomainInputDefFree; -virDomainIOThreadsPinDel; virDomainLeaseDefFree; virDomainLeaseIndex; virDomainLeaseInsert; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/12] Convert virDomainVcpuPinDefFree to virDomainPinDefFree
Make common between Vcpu and IOThreads Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 20 ++-- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 04de04e..db04855 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2037,7 +2037,7 @@ virDomainVcpuPinDefCopy(virDomainPinDefPtr *src, int nvcpupin) } void -virDomainVcpuPinDefFree(virDomainPinDefPtr def) +virDomainPinDefFree(virDomainPinDefPtr def) { if (def) { virBitmapFree(def-cpumask); @@ -2055,7 +2055,7 @@ virDomainVcpuPinDefArrayFree(virDomainPinDefPtr *def, return; for (i = 0; i nvcpupin; i++) -virDomainVcpuPinDefFree(def[i]); +virDomainPinDefFree(def[i]); VIR_FREE(def); } @@ -2231,7 +2231,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainVcpuPinDefArrayFree(def-cputune.vcpupin, def-cputune.nvcpupin); -virDomainVcpuPinDefFree(def-cputune.emulatorpin); +virDomainPinDefFree(def-cputune.emulatorpin); virDomainVcpuPinDefArrayFree(def-cputune.iothreadspin, def-cputune.niothreadspin); @@ -13474,7 +13474,7 @@ virDomainDefParseXML(xmlDocPtr xml, vcpupin-id)) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(duplicate vcpupin for same vcpu)); -virDomainVcpuPinDefFree(vcpupin); +virDomainPinDefFree(vcpupin); goto error; } @@ -13485,7 +13485,7 @@ virDomainDefParseXML(xmlDocPtr xml, * ignoring them instead. */ VIR_WARN(Ignore vcpupin for not onlined vcpus); -virDomainVcpuPinDefFree(vcpupin); +virDomainPinDefFree(vcpupin); } else { def-cputune.vcpupin[def-cputune.nvcpupin++] = vcpupin; } @@ -13566,7 +13566,7 @@ virDomainDefParseXML(xmlDocPtr xml, iothreadpin-id)) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(duplicate iothreadpin for same iothread)); -virDomainVcpuPinDefFree(iothreadpin); +virDomainPinDefFree(iothreadpin); goto error; } @@ -16751,7 +16751,7 @@ virDomainVcpuPinAdd(virDomainPinDefPtr **vcpupin_list, return 0; error: -virDomainVcpuPinDefFree(vcpupin); +virDomainPinDefFree(vcpupin); return -1; } @@ -16786,7 +16786,7 @@ virDomainEmulatorPinAdd(virDomainDefPtr def, emulatorpin-id = -1; emulatorpin-cpumask = virBitmapNewData(cpumap, maplen); if (!emulatorpin-cpumask) { -virDomainVcpuPinDefFree(emulatorpin); +virDomainPinDefFree(emulatorpin); return -1; } @@ -16810,7 +16810,7 @@ virDomainEmulatorPinDel(virDomainDefPtr def) if (!def-cputune.emulatorpin) return 0; -virDomainVcpuPinDefFree(def-cputune.emulatorpin); +virDomainPinDefFree(def-cputune.emulatorpin); def-cputune.emulatorpin = NULL; return 0; @@ -16857,7 +16857,7 @@ virDomainIOThreadsPinAdd(virDomainPinDefPtr **iothreadspin_list, return 0; error: -virDomainVcpuPinDefFree(iothreadpin); +virDomainPinDefFree(iothreadpin); return -1; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f7f1146..3f04832 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1903,7 +1903,7 @@ struct _virDomainPinDef { virBitmapPtr cpumask; }; -void virDomainVcpuPinDefFree(virDomainPinDefPtr def); +void virDomainPinDefFree(virDomainPinDefPtr def); void virDomainVcpuPinDefArrayFree(virDomainPinDefPtr *def, int nvcpupin); virDomainPinDefPtr *virDomainVcpuPinDefCopy(virDomainPinDefPtr *src, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 63e378b..adcb456 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -381,6 +381,7 @@ virDomainObjTaint; virDomainParseMemory; virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; +virDomainPinDefFree; virDomainPMSuspendedReasonTypeFromString; virDomainPMSuspendedReasonTypeToString; virDomainRedirdevBusTypeFromString; @@ -435,7 +436,6 @@ virDomainTPMModelTypeToString; virDomainVcpuPinAdd; virDomainVcpuPinDefArrayFree; virDomainVcpuPinDefCopy; -virDomainVcpuPinDefFree; virDomainVcpuPinDel; virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 94221fb..fed0c0d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5293,7 +5293,7 @@ qemuDomainPinEmulator(virDomainPtr dom, goto endjob; } } else { -virDomainVcpuPinDefFree(vm-def-cputune.emulatorpin); +
[libvirt] [PATCH 10/12] Replace virDomainIOThreadsPinAdd with virDomainPinAdd
This one is no longer necessary Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 45 - src/conf/domain_conf.h | 6 -- src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 14 +++--- 4 files changed, 7 insertions(+), 59 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4a9d083..19aa6f6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16814,51 +16814,6 @@ virDomainEmulatorPinDel(virDomainDefPtr def) return 0; } -int -virDomainIOThreadsPinAdd(virDomainPinDefPtr **iothreadspin_list, - size_t *niothreadspin, - unsigned char *cpumap, - int maplen, - unsigned int iothread_id) -{ -virDomainPinDefPtr iothreadpin = NULL; - -if (!iothreadspin_list) -return -1; - -iothreadpin = virDomainPinFindByVcpu(*iothreadspin_list, - *niothreadspin, - iothread_id); -if (iothreadpin) { -iothreadpin-id = iothread_id; -virBitmapFree(iothreadpin-cpumask); -iothreadpin-cpumask = virBitmapNewData(cpumap, maplen); -if (!iothreadpin-cpumask) -return -1; - -return 0; -} - -/* No existing iothreadpin matches iothread_id, adding a new one */ - -if (VIR_ALLOC(iothreadpin) 0) -goto error; - -iothreadpin-id = iothread_id; -iothreadpin-cpumask = virBitmapNewData(cpumap, maplen); -if (!iothreadpin-cpumask) -goto error; - -if (VIR_APPEND_ELEMENT(*iothreadspin_list, *niothreadspin, iothreadpin) 0) -goto error; - -return 0; - - error: -virDomainPinDefFree(iothreadpin); -return -1; -} - void virDomainIOThreadsPinDel(virDomainDefPtr def, unsigned int iothread_id) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 10fdbb0..1ba1ffe 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2563,12 +2563,6 @@ int virDomainEmulatorPinAdd(virDomainDefPtr def, int virDomainEmulatorPinDel(virDomainDefPtr def); -int virDomainIOThreadsPinAdd(virDomainPinDefPtr **iothreadspin_list, - size_t *niothreads, - unsigned char *cpumap, - int maplen, - unsigned int iothread_id); - void virDomainIOThreadsPinDel(virDomainDefPtr def, unsigned int iothread_id); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 54c931e..108e806 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -309,7 +309,6 @@ virDomainHubTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; virDomainInputDefFree; -virDomainIOThreadsPinAdd; virDomainIOThreadsPinDel; virDomainLeaseDefFree; virDomainLeaseIndex; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3e40724..db8cf4f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5852,8 +5852,8 @@ qemuDomainPinIOThread(virDomainPtr dom, newIOThreadsPinNum = 0; } -if (virDomainIOThreadsPinAdd(newIOThreadsPin, newIOThreadsPinNum, - cpumap, maplen, iothread_id) 0) { +if (virDomainPinAdd(newIOThreadsPin, newIOThreadsPinNum, +cpumap, maplen, iothread_id) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(failed to update iothreadspin)); goto endjob; @@ -5924,11 +5924,11 @@ qemuDomainPinIOThread(virDomainPtr dom, goto endjob; persistentDef-cputune.niothreadspin = 0; } -if (virDomainIOThreadsPinAdd(persistentDef-cputune.iothreadspin, - persistentDef-cputune.niothreadspin, - cpumap, - maplen, - iothread_id) 0) { +if (virDomainPinAdd(persistentDef-cputune.iothreadspin, +persistentDef-cputune.niothreadspin, +cpumap, +maplen, +iothread_id) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(failed to update or add iothreadspin xml of a persistent domain)); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/12] Convert virDomainVcpuPinFindByVcpu into virDomainPinFindByVcpu
Make common between Vcpu and IOThreads Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 24 src/conf/domain_conf.h | 6 +++--- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 12 ++-- src/qemu/qemu_process.c | 12 ++-- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index efc01bd..75d75bc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16693,17 +16693,17 @@ virDomainPinIsDuplicate(virDomainPinDefPtr *def, } virDomainPinDefPtr -virDomainVcpuPinFindByVcpu(virDomainPinDefPtr *def, - int nvcpupin, - int vcpu) +virDomainPinFindByVcpu(virDomainPinDefPtr *def, + int npin, + int id) { size_t i; -if (!def || !nvcpupin) +if (!def || !npin) return NULL; -for (i = 0; i nvcpupin; i++) { -if (def[i]-id == vcpu) +for (i = 0; i npin; i++) { +if (def[i]-id == id) return def[i]; } @@ -16722,9 +16722,9 @@ virDomainVcpuPinAdd(virDomainPinDefPtr **vcpupin_list, if (!vcpupin_list) return -1; -vcpupin = virDomainVcpuPinFindByVcpu(*vcpupin_list, - *nvcpupin, - vcpu); +vcpupin = virDomainPinFindByVcpu(*vcpupin_list, + *nvcpupin, + vcpu); if (vcpupin) { vcpupin-id = vcpu; virBitmapFree(vcpupin-cpumask); @@ -16828,9 +16828,9 @@ virDomainIOThreadsPinAdd(virDomainPinDefPtr **iothreadspin_list, if (!iothreadspin_list) return -1; -iothreadpin = virDomainVcpuPinFindByVcpu(*iothreadspin_list, - *niothreadspin, - iothread_id); +iothreadpin = virDomainPinFindByVcpu(*iothreadspin_list, + *niothreadspin, + iothread_id); if (iothreadpin) { iothreadpin-id = iothread_id; virBitmapFree(iothreadpin-cpumask); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0fb8d01..fe61b9f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1913,9 +1913,9 @@ int virDomainPinIsDuplicate(virDomainPinDefPtr *def, int npin, int id); -virDomainPinDefPtr virDomainVcpuPinFindByVcpu(virDomainPinDefPtr *def, - int nvcpupin, - int vcpu); +virDomainPinDefPtr virDomainPinFindByVcpu(virDomainPinDefPtr *def, + int npin, + int id); typedef struct _virBlkioDevice virBlkioDevice; typedef virBlkioDevice *virBlkioDevicePtr; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 365430f..540936f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -384,6 +384,7 @@ virDomainPausedReasonTypeToString; virDomainPinDefArrayFree; virDomainPinDefCopy; virDomainPinDefFree; +virDomainPinFindByVcpu; virDomainPinIsDuplicate; virDomainPMSuspendedReasonTypeFromString; virDomainPMSuspendedReasonTypeToString; @@ -438,7 +439,6 @@ virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; virDomainVcpuPinAdd; virDomainVcpuPinDel; -virDomainVcpuPinFindByVcpu; virDomainVideoDefaultRAM; virDomainVideoDefaultType; virDomainVideoDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0d6f367..5d867c3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5143,9 +5143,9 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, unsigned char *tmpmap = NULL; int tmpmaplen; -pininfo = virDomainVcpuPinFindByVcpu(targetDef-cputune.vcpupin, - targetDef-cputune.nvcpupin, - vcpu); +pininfo = virDomainPinFindByVcpu(targetDef-cputune.vcpupin, + targetDef-cputune.nvcpupin, + vcpu); if (!pininfo) { if (!(bitmap = virBitmapNew(hostcpus))) goto cleanup; @@ -5679,9 +5679,9 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, info_ret[i]-iothread_id = i + 1; /* Initialize the cpumap */ -pininfo = virDomainVcpuPinFindByVcpu(targetDef-cputune.iothreadspin, - targetDef-cputune.niothreadspin, - i + 1); +pininfo = virDomainPinFindByVcpu(targetDef-cputune.iothreadspin, + targetDef-cputune.niothreadspin, +
[libvirt] [PATCH 11/12] Replace virDomainVcpuPinDel with virDomainPinDel
Make common between Vcpu and IOThreads Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 15 --- src/conf/domain_conf.h | 4 +++- src/libvirt_private.syms | 2 +- src/libxl/libxl_driver.c | 4 +++- src/qemu/qemu_driver.c | 16 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 19aa6f6..24c0d8e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16754,16 +16754,17 @@ virDomainPinAdd(virDomainPinDefPtr **pindef_list, } void -virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) +virDomainPinDel(virDomainPinDefPtr **pindef_list, +size_t *npin, +int id) { int n; -virDomainPinDefPtr *vcpupin_list = def-cputune.vcpupin; -for (n = 0; n def-cputune.nvcpupin; n++) { -if (vcpupin_list[n]-id == vcpu) { -virBitmapFree(vcpupin_list[n]-cpumask); -VIR_FREE(vcpupin_list[n]); -VIR_DELETE_ELEMENT(def-cputune.vcpupin, n, def-cputune.nvcpupin); +for (n = 0; n *npin; n++) { +if ((*pindef_list)[n]-id == id) { +virBitmapFree((*pindef_list)[n]-cpumask); +VIR_FREE((*pindef_list)[n]); +VIR_DELETE_ELEMENT(*pindef_list, n, *npin); return; } } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1ba1ffe..c10e080 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2555,7 +2555,9 @@ int virDomainPinAdd(virDomainPinDefPtr **pindef_list, int maplen, int id); -void virDomainVcpuPinDel(virDomainDefPtr def, int vcpu); +void virDomainPinDel(virDomainPinDefPtr **pindef_list, + size_t *npin, + int vcpu); int virDomainEmulatorPinAdd(virDomainDefPtr def, unsigned char *cpumap, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 108e806..c453fb8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -384,6 +384,7 @@ virDomainPinAdd; virDomainPinDefArrayFree; virDomainPinDefCopy; virDomainPinDefFree; +virDomainPinDel; virDomainPinFindByVcpu; virDomainPinIsDuplicate; virDomainPMSuspendedReasonTypeFromString; @@ -437,7 +438,6 @@ virDomainTPMBackendTypeToString; virDomainTPMDefFree; virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; -virDomainVcpuPinDel; virDomainVideoDefaultRAM; virDomainVideoDefaultType; virDomainVideoDefFree; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d1addf5..f738d8f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1990,7 +1990,9 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, /* full bitmap means reset the settings (if any). */ if (virBitmapIsAllSet(pcpumap)) { -virDomainVcpuPinDel(targetDef, vcpu); +virDomainPinDel(targetDef-cputune.vcpupin, +targetDef-cputune.nvcpupin, +vcpu); goto done; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index db8cf4f..e4b6799 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4656,7 +4656,9 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, } /* Free vcpupin setting */ -virDomainVcpuPinDel(vm-def, i); +virDomainPinDel(vm-def-cputune.vcpupin, +vm-def-cputune.nvcpupin, +i); } } @@ -4838,7 +4840,9 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, /* remove vcpupin entries for vcpus that were unplugged */ if (nvcpus persistentDef-vcpus) { for (i = persistentDef-vcpus; i = nvcpus; i--) -virDomainVcpuPinDel(persistentDef, i); +virDomainPinDel(persistentDef-cputune.vcpupin, +persistentDef-cputune.nvcpupin, +i); } if (maximum) { @@ -5006,7 +5010,9 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, } if (doReset) { -virDomainVcpuPinDel(vm-def, vcpu); +virDomainPinDel(vm-def-cputune.vcpupin, +vm-def-cputune.nvcpupin, +vcpu); } else { if (vm-def-cputune.vcpupin) virDomainPinDefArrayFree(vm-def-cputune.vcpupin, @@ -5036,7 +5042,9 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, if (flags VIR_DOMAIN_AFFECT_CONFIG) { if (doReset) { -virDomainVcpuPinDel(persistentDef, vcpu); +virDomainPinDel(persistentDef-cputune.vcpupin, +persistentDef-cputune.nvcpupin, +vcpu); } else { if (!persistentDef-cputune.vcpupin) { if
Re: [libvirt] [PATCH] network: avoid memory leak of dnsmasq capabilities
On 03/13/2015 06:23 PM, Eric Blake wrote: Valgrind detected a leak: ==17820== 102 (56 direct, 46 indirect) bytes in 1 blocks are definitely lost in loss record 479 of 646 ==17820==at 0x4A08946: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==17820==by 0x508521A: virAllocVar (viralloc.c:560) ==17820==by 0x50D9FCA: virObjectNew (virobject.c:193) ==17820==by 0x50A4FD9: dnsmasqCapsNewEmpty (virdnsmasq.c:784) ==17820==by 0x50A514E: dnsmasqCapsNewFromBinary (virdnsmasq.c:830) ==17820==by 0x1B508287: networkStateInitialize (bridge_driver.c:666) It looks like commit 172acef introduced the problem, because networkGetDnsmasqCaps() increments the reference count but an early exit never does a matching decrement. * src/network/bridge_driver.c (networkStateCleanup): Plug leak. Signed-off-by: Eric Blake ebl...@redhat.com --- src/network/bridge_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix report of non-active commit completion
On 03/13/2015 12:04 PM, Eric Blake wrote: Commit f182da20 (v1.2.6) caused a slight regression in virsh reporting of a non-active block job; where it used to state Commit complete, it now states Now in synchronized phase. But the synchronized phase is only possible for an active commit. For a reproducer, I created a chain 'a - b - c - d - e' and ran virsh blockcommit $dom vda --top c --base a --verbose --wait * tools/virsh-domain.c (cmdBlockCommit): Synchronized phase is only possible on active commits. Signed-off-by: Eric Blake ebl...@redhat.com --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] libxl: Stubdom emulator type
On Fri, Mar 13, 2015 at 02:09:34PM -0600, Jim Fehlig wrote: Marek Marczykowski-Górecki wrote: Xen have feature of having device model in separate domain (called stub domain). Add stubdomain element to allow selecting such configuration. Emulator path is still used for qemu running in dom0 (if any). Libxl currently do not allow to select stubdomain path. Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- docs/formatdomain.html.in | 13 + docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c| 14 ++ src/conf/domain_conf.h| 3 ++- src/libxl/libxl_conf.c| 8 5 files changed, 47 insertions(+), 1 deletion(-) Changes in v4: - change emulator type='stubdom' to separate stubdomain element diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fb0a0d1..054f48f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1670,6 +1670,19 @@ /dd /dl +dl + dtcodestubdomain/code/dt + dd +span class=sinceSince 1.2.13/span, the optional codestubdomain/code + element contains codeenabled/code yes/no/default attribute. If enabled + Emulator will be launched in stub domain (Xen only). Currently there + is no way to provide path to that emulator. Note that in most cases + this emulator will be in addition to one in dom0. Why does a stubdomain need an emulator (running in dom0) to serve as a backend? Wouldn't it be better to give the stubdomain direct access to the resources it needs? Otherwise, It seems to defeat stubdomain's purpose of moving the emulator execution out of dom0. At least the Xen wiki describes this as primary motivation for studomains http://wiki.xen.org/wiki/Device_Model_Stub_Domains Two reasons: 1. To host video/keyboard (vfb/vkb) backends for stubdomain - stubdomain have no simple way to expose VNC server to the network. 2. To host a console backend. Xenconsoled supports only one console per domain, but qemu in stubdomain needs at least three (logs, pipe to save the state and another one to restore the state during save/restore). Actually in Qubes OS we have our own GUI agent, so the first point is not needed. And we broke the second one just to get rid of qemu in dom0... I'll be more than happy if/when xenconsoled will get support for more than one console device per domain. + codeemulator/code element described above still controls the path + to the dom0 instance. + /dd +/dl + I think it would be good to add an example above the description. E.g. along the lines of Daniel's previous example devices emulator/usr/lib/xen/bin/qemu-system-i386/emulator stubdomain enabled=yes|no/ /devices Ok, I'll add it in the next patch version (if any). Regards, Jim h4a name=elementsDisksHard drives, floppy disks, CDROMs/a/h4 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4e4fe9f..539db39 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2567,6 +2567,13 @@ ref name=absFilePath/ /element /define + define name=stubdomain +element name=stubdomain + attribute name=enabled +ref name=virYesNo/ + /attribute +/element + /define !-- A graphic description, currently in Xen only 2 types are supported: - sdl with optional display, xauth and fullscreen @@ -4007,6 +4014,9 @@ optional ref name=emulator/ /optional +optional + ref name=stubdomain/ +/optional zeroOrMore choice ref name=disk/ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c430e03..e327e55 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14107,6 +14107,9 @@ virDomainDefParseXML(xmlDocPtr xml, } def-emulator = virXPathString(string(./devices/emulator[1]), ctxt); +tmp = virXPathString(string(./devices/stubdomain/@enabled), ctxt); +if (tmp) +def-stubdomain = virTristateBoolTypeFromString(tmp); /* analysis of the disk devices */ if ((n = virXPathNodeSet(./devices/disk, ctxt, nodes)) 0) @@ -16064,6 +16067,14 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; } +if (src-stubdomain != dst-stubdomain) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, +_(Target domain stubdomain '%s' does not match source '%s'), +virTristateBoolTypeToString(dst-stubdomain), +virTristateBoolTypeToString(src-stubdomain)); +goto error; +} + if (!virDomainDefFeaturesCheckABIStability(src, dst))
[libvirt] [PATCH] libxl: fix regression introduced by commit 4ab8cd77
Commit 4ab8cd77 added a check requiring input devices to have a bus type of VIR_DOMAIN_INPUT_BUS_USB, failing to start the domain otherwise. But virDomainDefParseXML adds implicit mouse and keyboard if a graphics device is configured. See calls to virDomainDefMaybeAddInput. The regression is fixed by removing the check requiring USB input devices, and skipping non-USB input devices when populating USB 'usbdevice' in libxl_domain_build_info struct. Signed-off-by: Jim Fehlig jfeh...@suse.com --- src/libxl/libxl_conf.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 50ef9d8..2b57d0b 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -750,13 +750,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_defbool_set(b_info-u.hvm.sdl.enable, 0); if (def-ninputs) { -for (i = 0; i def-ninputs; i++) { -if (def-inputs[i]-bus != VIR_DOMAIN_INPUT_BUS_USB) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, -_(libxenlight supports only USB input)); -return -1; -} -} #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST if (VIR_ALLOC_N(b_info-u.hvm.usbdevice_list, def-ninputs+1) 0) return -1; @@ -769,6 +762,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, #endif for (i = 0; i def-ninputs; i++) { char **usbdevice; + +if (def-inputs[i]-bus != VIR_DOMAIN_INPUT_BUS_USB) +continue; + #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST usbdevice = b_info-u.hvm.usbdevice_list[i]; #else -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 08/10] qemu: conf: Add support for memory device cold(un)plug
+ +int +virDomainMemoryInsert(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ +int id = def-nmems; + +if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE +virDomainDefHasDeviceAddress(def, mem-info)) { Hmm... so if our incoming mem has an address defined - it could be anything - we're just failing declaring that the domain already contains a device with the same address? Doesn't seem right. And again - we have this problem of TYPE_NONE, TYPE_DIMM, TYPE_LAST and who knows what in the future. Perhaps I was thinking too much about the various Find* API's when I first read this, but upon further reflection and looking at the code paths using it I see what the goal is. So safely ignore this particular comment. John +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(Domain already contains a device with the same + address)); +return -1; +} + +if (VIR_APPEND_ELEMENT(def-mems, def-nmems, mem) 0) +return -1; + +return id; +} + -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 09/10] qemu: Implement memory device hotplug
On 03/04/2015 11:25 AM, Peter Krempa wrote: Add code to hot-add memory devices to running qemu instances. --- src/qemu/qemu_command.c | 4 +-- src/qemu/qemu_command.h | 15 + src/qemu/qemu_driver.c | 5 ++- src/qemu/qemu_hotplug.c | 85 + src/qemu/qemu_hotplug.h | 3 ++ 5 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f1fca02..883c3d1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4546,7 +4546,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, * other configuration was used (to detect legacy configurations). Returns * -1 in case of an error. */ -static int +int qemuBuildMemoryBackendStr(unsigned long long size, unsigned long long pagesize, int guestNode, @@ -4819,7 +4819,7 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, } -static char * +char * qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, virQEMUCapsPtr qemuCaps) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index ee81f92..a29db41 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -162,6 +162,21 @@ char *qemuBuildSoundDevStr(virDomainDefPtr domainDef, virDomainSoundDefPtr sound, virQEMUCapsPtr qemuCaps); +int qemuBuildMemoryBackendStr(unsigned long long size, + unsigned long long pagesize, + int guestNode, + virBitmapPtr userNodeset, + virBitmapPtr autoNodeset, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virQEMUDriverConfigPtr cfg, + const char **backendType, + virJSONValuePtr *backendProps, + bool force); + +char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, + virQEMUCapsPtr qemuCaps); + /* Legacy, pre device support */ char *qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6b10e96..b917d76 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7095,8 +7095,11 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, dev-data.rng = NULL; break; -/*TODO: implement later */ case VIR_DOMAIN_DEVICE_MEMORY: +ret = qemuDomainAttachMemory(driver, vm, + dev-data.memory); Shouldn't there be a : if (!ret) prior to the following line (like the other cases in the switch) +dev-data.memory = NULL; +break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_FS: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cb2270e..967b678 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1672,6 +1672,91 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, } +int +qemuDomainAttachMemory(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +char *devstr = NULL; +char *objalias = NULL; +const char *backendType; +virJSONValuePtr props = NULL; +int id; +int ret = -1; + +if (virAsprintf(mem-info.alias, dimm%zu, vm-def-nmems) 0) +goto cleanup; + +if (virAsprintf(objalias, mem%s, mem-info.alias) 0) +goto cleanup; + +if (!(devstr = qemuBuildMemoryDeviceStr(mem, priv-qemuCaps))) +goto cleanup; + +qemuDomainMemoryDeviceAlignSize(mem); + +if (qemuBuildMemoryBackendStr(mem-size, mem-pagesize, + mem-targetNode, mem-sourceNodes, NULL, + vm-def, priv-qemuCaps, cfg, + backendType, props, true) 0) +goto cleanup; + +if (virDomainMemoryInsert(vm-def, mem) 0) { +virJSONValueFree(props); +goto cleanup; +} + +qemuDomainObjEnterMonitor(driver, vm); +if (qemuMonitorAddObject(priv-mon, backendType, objalias, props) 0) I see from the AddObject comments, props is consumed upon success, but if we hit the else of mon-json, then we don't free it which we should - not related to this particular code, but something that should be fixed... +goto removedef; + +if (qemuMonitorAddDevice(priv-mon, devstr) 0) { +virErrorPtr err = virSaveLastError(); +
Re: [libvirt] [PATCHv2.5 10/10] qemu: Implement memory device hotunplug
On 03/04/2015 11:25 AM, Peter Krempa wrote: Add code to hot-remove memory devices from qemu. Unfortunately QEMU doesn't support this right now, so this is just for completenes. --- src/qemu/qemu_driver.c | 4 ++- src/qemu/qemu_hotplug.c | 91 - src/qemu/qemu_hotplug.h | 3 ++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b917d76..cd9c0f2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7179,7 +7179,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, ret = qemuDomainDetachRNGDevice(driver, vm, dev-data.rng); break; case VIR_DOMAIN_DEVICE_MEMORY: -/* TODO: Implement later */ +ret = qemuDomainDetachMemoryDevice(driver, vm, dev-data.memory); +break; + case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 967b678..359fb8e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2818,6 +2818,44 @@ qemuDomainRemoveControllerDevice(virQEMUDriverPtr driver, } +static int +qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr mem) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virObjectEventPtr event; +char *backendAlias = NULL; +int rc; +int idx; + +VIR_DEBUG(Removing memory device %s from domain %p %s, + mem-info.alias, vm, vm-def-name); + +if ((event = virDomainEventDeviceRemovedNewFromObj(vm, mem-info.alias))) +qemuDomainEventQueue(driver, event); + +if (virAsprintf(backendAlias, mem%s, mem-info.alias) 0) +goto error; + +qemuDomainObjEnterMonitor(driver, vm); +rc = qemuMonitorDelObject(priv-mon, backendAlias); +if (qemuDomainObjExitMonitor(driver, vm) 0 || rc 0) +goto error; + +if ((idx = virDomainMemoryFindByDef(vm-def, mem)) = 0) +virDomainMemoryRemove(vm-def, idx); + +virDomainMemoryDefFree(mem); +VIR_FREE(backendAlias); +return 0; + + error: +VIR_FREE(backendAlias); +return -1; +} + + static void qemuDomainRemovePCIHostDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3155,8 +3193,9 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, qemuDomainRemoveRNGDevice(driver, vm, dev-data.rng); break; -/* TODO: implement later */ case VIR_DOMAIN_DEVICE_MEMORY: +ret = qemuDomainRemoveMemoryDevice(driver, vm, dev-data.memory); +break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LEASE: @@ -4108,3 +4147,53 @@ qemuDomainDetachRNGDevice(virQEMUDriverPtr driver, qemuDomainResetDeviceRemoval(vm); return ret; } + + +int +qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMemoryDefPtr memdef) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virDomainMemoryDefPtr mem; +int idx; +int rc; +int ret = -1; + +if ((idx = virDomainMemoryFindByDef(vm-def, memdef)) 0) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(device not present in domain configuration)); +return -1; +} + +mem = vm-def-mems[idx]; + +if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(qemu does not support -device)); +return -1; +} This check could be first to save a cycle of searching FindByDef Not that it matters because if we get this far, one would hope -device was present! ACK - John . + +if (!mem-info.alias) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(alias for the memory device was not found)); +return -1; +} + +qemuDomainMarkDeviceForRemoval(vm, mem-info); + +qemuDomainObjEnterMonitor(driver, vm); +rc = qemuMonitorDelDevice(priv-mon, mem-info.alias); +if (qemuDomainObjExitMonitor(driver, vm) 0 || rc 0) +goto cleanup; + +rc = qemuDomainWaitForDeviceRemoval(vm); +if (rc == 0 || rc == 1) +ret = qemuDomainRemoveMemoryDevice(driver, vm, mem); +else +ret = 0; + + cleanup: +qemuDomainResetDeviceRemoval(vm); +return ret; +} diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index ad4ff38..4140da3 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -60,6 +60,9 @@ int qemuDomainFindGraphicsIndex(virDomainDefPtr def, int qemuDomainAttachMemory(virQEMUDriverPtr driver, virDomainObjPtr vm,
Re: [libvirt] [libvirt-test-API][PATCH 2/2] Add coredump_with_format test case to linux_domain conf
ACK and Pushed Thanks Hongming On 02/27/2015 01:52 PM, jiahu wrote: --- cases/linux_domain.conf | 44 1 file changed, 44 insertions(+) diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf index a5ada35..490ee90 100644 --- a/cases/linux_domain.conf +++ b/cases/linux_domain.conf @@ -34,6 +34,50 @@ domain:start guestname $defaultname +domain:coredump_with_format +guestname +$defaultname +topath +/root/test.dump +dumpformat +zlib +flags +mem + +domain:coredump_with_format +guestname +$defaultname +topath +/root/test.dump +dumpformat +raw +flags +mem|live|bypass + +domain:coredump_with_format +guestname +$defaultname +topath +/root/test.dump +dumpformat +snappy +flags +mem|reset + +domain:coredump_with_format +guestname +$defaultname +topath +/root/test.dump +dumpformat +lzo +flags +mem|crash|bypass + +domain:start +guestname +$defaultname + domain:destroy guestname $defaultname -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: vhost user: support for bootindex
Problem Description: When we set boot order for a vhost-user network interface, we found the boot index doesn't work. Cause of the Problem: In the function qemuBuildVhostuserCommandLine(), it forcely set the arg bootindex of function qemuBuildNicDevStr() to 0. Thus, the bootindex parameter got missing. Solution: Trans the arg bootindex down. Signed-off-by: Gao Haifeng gaohaifeng@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/qemu/qemu_command.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5303de5..2f37812 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7767,7 +7767,8 @@ static int qemuBuildVhostuserCommandLine(virCommandPtr cmd, virDomainDefPtr def, virDomainNetDefPtr net, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps, + int bootindex) { virBuffer chardev_buf = VIR_BUFFER_INITIALIZER; virBuffer netdev_buf = VIR_BUFFER_INITIALIZER; @@ -7814,7 +7815,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, -netdev); virCommandAddArgBuffer(cmd, netdev_buf); -if (!(nic = qemuBuildNicDevStr(def, net, -1, 0, 0, qemuCaps))) { +if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex, 0, qemuCaps))) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Error generating NIC -device string)); goto error; @@ -7859,8 +7860,12 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, virNetDevBandwidthPtr actualBandwidth; size_t i; + +if (!bootindex) +bootindex = net-info.bootIndex; + if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) -return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps); +return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps, bootindex); if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { /* NET_TYPE_HOSTDEV devices are really hostdev devices, so @@ -7869,9 +7874,6 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, return 0; } -if (!bootindex) -bootindex = net-info.bootIndex; - /* Currently nothing besides TAP devices supports multiqueue. */ if (net-driver.virtio.queues 0 !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/7] Drop network driver lock
On Thu, Mar 12, 2015 at 15:39:14 +0100, Michal Privoznik wrote: Hopefully, the last version. Again, some patches are ACKed already, but I'm sending them again. Not to trash the review bandwidth, but for reviewer to get better picture. Patches 5-7 are already ACKed. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 4/4] migration: Expose 'cancelling' status to user
On 09/03/2015 07:45, zhanghailiang wrote: 'cancelling' status was introduced by commit 51cf4c1a, mainly to avoid a possible start of a new migration process while the previous one still exists. But we didn't expose this status to user, instead we returned the 'active' state. Here, we expose it to the user (such as libvirt), 'cancelling' status only occurs for a short window before the migration aborts, so for users, if they cancel a migration process, it will observe 'cancelling' status occasionally. Testing revealed that with older libvirt (anything 1.2.13 or less) will print an odd error message if the state is seen, but that the migration is still properly cancelled. Newer libvirt will be patched to recognize the new state without the odd error message. Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com Reviewed-by: Eric Blake ebl...@redhat.com Cc: libvir-list@redhat.com Why is this necessary? Paolo --- migration/migration.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 035e005..a57928d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -179,13 +179,11 @@ MigrationInfo *qmp_query_migrate(Error **errp) break; case MIGRATION_STATUS_SETUP: info-has_status = true; -info-status = MIGRATION_STATUS_SETUP; info-has_total_time = false; break; case MIGRATION_STATUS_ACTIVE: case MIGRATION_STATUS_CANCELLING: info-has_status = true; -info-status = MIGRATION_STATUS_ACTIVE; info-has_total_time = true; info-total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - s-total_time; @@ -221,7 +219,6 @@ MigrationInfo *qmp_query_migrate(Error **errp) get_xbzrle_cache_stats(info); info-has_status = true; -info-status = MIGRATION_STATUS_COMPLETED; info-has_total_time = true; info-total_time = s-total_time; info-has_downtime = true; @@ -243,13 +240,12 @@ MigrationInfo *qmp_query_migrate(Error **errp) break; case MIGRATION_STATUS_FAILED: info-has_status = true; -info-status = MIGRATION_STATUS_FAILED; break; case MIGRATION_STATUS_CANCELLED: info-has_status = true; -info-status = MIGRATION_STATUS_CANCELLED; break; } +info-status = s-state; return info; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: fix use the wrong type for period
https://bugzilla.redhat.com/show_bug.cgi?id=1140958 When we set period as unsigned int max value 4294967295 and start the vm, qemu will report error. This becuase we define period as a unsigned int and parse it as a unsigned int, but we use it as a int when set it via QMP in qemuMonitorJSONSetMemoryStatsPeriod, so 4294967295 turn to -1 when we send the QMP command. After check the qemu's code this value type should be int, and found a qemu commit 1f9296b for this values range. Seems no other hypervisor vm use this so i add a check when we parse it. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 127fc91..54bd5aa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10452,7 +10452,8 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, } ctxt-node = node; -if (virXPathUInt(string(./stats/@period), ctxt, def-period) -1) { +if (virXPathInt(string(./stats/@period), ctxt, def-period) -1 || +def-period 0) { virReportError(VIR_ERR_XML_ERROR, %s, _(invalid statistics collection period)); goto error; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ea463cb..ee0f5fd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1556,7 +1556,7 @@ enum { struct _virDomainMemballoonDef { int model; virDomainDeviceInfo info; -unsigned int period; /* seconds between collections */ +int period; /* seconds between collections */ }; struct _virDomainNVRAMDef { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] logging: how about adding a ProcessName field in logging file?
On Fri, Mar 13, 2015 at 05:08:54PM +0800, zhang bo wrote: Suppose there are 3 or more clients of libvirt: 1)nova 2)bash virsh commands 3)user customized ELF 4)etc The env LIBVIRT_DEBUG and LIBVIRT_LOG_OUTPUTS affects all of these clients, thus, they will all accumulate the logs into *ONE* file set by LIBVIRT_LOG_OUTPUTS. There is no attempt to make sure that separate clients logging to the same file will atomically write log lines. You could get half a line of text from one client, then half a line of text from a second client, then the rest of the line from the first client all mangled up. You simply shouldn't give each client process the same logging output file. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/7] network_driver: Use accessor for dnsmasqCaps
On Thu, Mar 12, 2015 at 15:39:16 +0100, Michal Privoznik wrote: This is not an immutable pointer and can change during lifetime. Therefore, in order to drop network driver lock, we must use an internal accessor which does not lock the network driver yet, but it will soon. Now it merely returns an referenced object. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/network/bridge_driver.c | 42 -- 1 file changed, 36 insertions(+), 6 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Should logging in the library set its default outputs?
If the env LIBVIRT_DEBUG and LIBVIRT_LOG_OUTPUTS are not set, the logging in the library would not log into any file. So: Is it necessary to set the default level and outputs in virGlobalInit(), just in case the maintainer forgets to set the ENVs ? Thanks in advance. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: read backing chain names from qemu
On Thu, Mar 12, 2015 at 14:23:48 -0600, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that after a series of disk snapshots into existing destination images, followed by active commits of the top image, it is possible for qemu 2.2 and earlier to end up tracking a different name for the image than what it would have had when opening the chain afresh. That is, when starting with the chain 'a - b - c', the name associated with 'b' is how it was spelled in the metadata of 'c', but when starting with 'a', taking two snapshots into 'a - b - c', then committing 'c' back into 'b', the name associated with 'b' is now the name used when taking the first snapshot. Sadly, older qemu doesn't know how to treat different spellings of the same filename as identical files (it uses strcmp() instead of checking for the same inode), which means libvirt's attempt to commit an image using solely the names learned from qcow2 metadata fails with a cryptic: error: internal error: unable to execute QEMU command 'block-commit': Top image file /tmp/images/c/../b/b not found even though the file exists. Trying to teach libvirt the rules on which name qemu will expect is not worth the effort (besides, we'd have to remember it across libvirtd restarts, and track whether a file was opened via metadata or via snapshot creation for a given qemu process); it is easier to just always directly ask qemu what string it expects to see in the first place. As a safety valve, we validate that any name returned by qemu still maps to the same local file as we have tracked it, so that a compromised qemu cannot accidentally cause us to act on an incorrect file. It would still allow to act on remote storage though. Also if qemu is corrupted in a way that it'd lie to us correctly via monitor it would be most probably also able to act on the file itself. As the labelling is done from the internal structures it should not allow to do anything besides what the instance is already allowed. A bigger problem though would be that since we don't store the backing chain internally all the time, qemu could rewrite the metadata in the image and libvirt would happily accept those. Corrupting qemu in that way is very unprobable though IMO. * src/qemu/qemu_monitor.h (qemuMonitorDiskNameLookup): New prototype. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskNameLookup): Likewise. * src/qemu/qemu_monitor.c (qemuMonitorDiskNameLookup): New function. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskNameLookup) (qemuMonitorJSONDiskNameLookupOne): Likewise. * src/qemu/qemu_driver.c (qemuDomainBlockCommit) (qemuDomainBlockJobImpl): Use it. Signed-off-by: Eric Blake ebl...@redhat.com --- v2: as suggested by Dan, add a sanity checking valve to ensure we don't use qemu's string until vetting that it resolves to the same local name we are already tracking src/qemu/qemu_driver.c | 28 ++--- src/qemu/qemu_monitor.c | 20 - src/qemu/qemu_monitor.h | 8 +++- src/qemu/qemu_monitor_json.c | 97 +++- src/qemu/qemu_monitor_json.h | 9 +++- 5 files changed, 144 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3263ac..f0e530d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c ... @@ -16172,8 +16169,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, } qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath, - speed, mode, async); +if (baseSource) +basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src, I remember that at some point accessing of domain definition while in the monitor was not okay for some reason, but I can't now remember why nor whether it was fixed. + baseSource); +if (!baseSource || basePath) +ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath, + speed, mode, async); if (qemuDomainObjExitMonitor(driver, vm) 0) ret = -1; if (ret 0) { ... diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d869a72..cf7dc5e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1,7 +1,7 @@ /* * qemu_monitor.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. Shouldn't we employ something as in gnulib, where copyrights would be bumped at once everywhere? * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or ... diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b30da34..e67d800 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1,7 +1,7 @@ /* *
[libvirt] logging: how about adding a ProcessName field in logging file?
Suppose there are 3 or more clients of libvirt: 1)nova 2)bash virsh commands 3)user customized ELF 4)etc The env LIBVIRT_DEBUG and LIBVIRT_LOG_OUTPUTS affects all of these clients, thus, they will all accumulate the logs into *ONE* file set by LIBVIRT_LOG_OUTPUTS. eg: [2015-03-07 00:33:30]: 103674: info : virDomainShutdown:3242 : enter virDomainShutdown domainname=VMName [2015-03-07 00:33:41]: 103674: info : virDomainShutdown:3253 : domain VMName shutted down [2015-03-13 00:53:44]: 5073: info : libvirt version: 1.2.7 [2015-03-13 00:53:44]: 5034: info : libvirt version: 1.2.7 [2015-03-13 00:53:44]: 5073: error : virNetSocketReadWire:1475 : End of file while reading data: Input/output error note: 103674: bash virsh command 5037: nova if we don't know that 103674 is just a virsh command, and suspect that it's nova, time would be wasted to find out who's the criminal. The improved log would be: [2015-03-07 00:33:30]: virsh: 103674: info : virDomainShutdown:3242 : enter virDomainShutdown domainname=VMName [2015-03-07 00:33:41]: virsh: 103674: info : virDomainShutdown:3253 : domain VMName shutted down [2015-03-13 00:53:44]: nova: 5073: info : libvirt version: 1.2.7 [2015-03-13 00:53:44]: myProc1: 5034: info : libvirt version: 1.2.7 [2015-03-13 00:53:44]: nova: error : virNetSocketReadWire:1475 : End of file while reading data: Input/output error So, here's the qeustion: Is it neccssary to add a ProcessName field in the logging file? if so, I'd like to apply a patch for this. thank you in advance. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] logging: how about adding a ProcessName field in logging file?
On 2015/3/13 17:29, Daniel P. Berrange wrote: On Fri, Mar 13, 2015 at 05:08:54PM +0800, zhang bo wrote: Suppose there are 3 or more clients of libvirt: 1)nova 2)bash virsh commands 3)user customized ELF 4)etc The env LIBVIRT_DEBUG and LIBVIRT_LOG_OUTPUTS affects all of these clients, thus, they will all accumulate the logs into *ONE* file set by LIBVIRT_LOG_OUTPUTS. There is no attempt to make sure that separate clients logging to the same file will atomically write log lines. You could get half a line of text from one client, then half a line of text from a second client, then the rest of the line from the first client all mangled up. You simply shouldn't give each client process the same logging output file. Regards, Daniel Got it, thank you for your immediate reply :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-API][PATCH V2 0/2] Add connection_getAllDomainStats test case
ACK and Pushed Thanks Hongming On 03/12/2015 03:10 PM, jiahu wrote: The testing case will validate the getAllDomainStats API in class virConnect V2: Added new domainListGetStats API in this case jiahu (2): Add connection_getAllDomainStats test case to linux_domain.conf Add connection_getAllDomainStats test case cases/linux_domain.conf | 14 + repos/virconn/connection_getAllDomainStats.py | 549 ++ 2 files changed, 563 insertions(+) create mode 100644 repos/virconn/connection_getAllDomainStats.py -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] parallels: fix prlsdkCheckUnsupportedParams checks
On Thu, Mar 12, 2015 at 18:43:29 +0300, Maxim Nestratov wrote: for memory limits since unset ones are no longer zero Ah, sorry, my bad for not noticing this while reviewing Pavel's series. Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index a775348..4c90a18 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1791,10 +1791,10 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def) } if (def-mem.nhugepages || -def-mem.hard_limit || -def-mem.soft_limit || +virMemoryLimitIsSet(def-mem.hard_limit) || +virMemoryLimitIsSet(def-mem.soft_limit) || def-mem.min_guarantee || -def-mem.swap_hard_limit) { +virMemoryLimitIsSet(def-mem.swap_hard_limit)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Memory parameter is not supported ACK, I'll push this once the build with this patch finishes. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/7] bridge_driver: Don't access global driver randomly
On Thu, Mar 12, 2015 at 15:39:15 +0100, Michal Privoznik wrote: Well, network driver code has the driver accessible as a global variable. This makes any rework hard, as it's unclear where the variable is accessed and/or modified. Lets just pass the driver as a parameter to all functions where needed. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/network/bridge_driver.c | 430 +--- 1 file changed, 247 insertions(+), 183 deletions(-) ACK, the renaming to network driver made it really easy to review. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/7] struct _virNetworkDriverState: Annotate items
On Thu, Mar 12, 2015 at 15:39:17 +0100, Michal Privoznik wrote: In order to drop network driver lock, lets annotate which structure items are immutable, which have self-locking APIs and so on. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/network/bridge_driver_platform.h | 7 +++ 1 file changed, 7 insertions(+) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 for-2.3 4/4] migration: Expose 'cancelling' status to user
'cancelling' status was introduced by commit 51cf4c1a, mainly to avoid a possible start of a new migration process while the previous one still exists. But we didn't expose this status to user, instead we returned the 'active' state. Here, we expose it to the user (such as libvirt), 'cancelling' status only occurs for a short window before the migration aborts, so for users, if they cancel a migration process, it will observe 'cancelling' status occasionally. Testing revealed that with older libvirt (anything 1.2.13 or less) will print an odd error message if the state is seen, but that the migration is still properly cancelled. Newer libvirt will be patched to recognize the new state without the odd error message. Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com Reviewed-by: Eric Blake ebl...@redhat.com Cc: libvir-list@redhat.com --- migration/migration.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 035e005..a57928d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -179,13 +179,11 @@ MigrationInfo *qmp_query_migrate(Error **errp) break; case MIGRATION_STATUS_SETUP: info-has_status = true; -info-status = MIGRATION_STATUS_SETUP; info-has_total_time = false; break; case MIGRATION_STATUS_ACTIVE: case MIGRATION_STATUS_CANCELLING: info-has_status = true; -info-status = MIGRATION_STATUS_ACTIVE; info-has_total_time = true; info-total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - s-total_time; @@ -221,7 +219,6 @@ MigrationInfo *qmp_query_migrate(Error **errp) get_xbzrle_cache_stats(info); info-has_status = true; -info-status = MIGRATION_STATUS_COMPLETED; info-has_total_time = true; info-total_time = s-total_time; info-has_downtime = true; @@ -243,13 +240,12 @@ MigrationInfo *qmp_query_migrate(Error **errp) break; case MIGRATION_STATUS_FAILED: info-has_status = true; -info-status = MIGRATION_STATUS_FAILED; break; case MIGRATION_STATUS_CANCELLED: info-has_status = true; -info-status = MIGRATION_STATUS_CANCELLED; break; } +info-status = s-state; return info; } -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] parallels: fix prlsdkCheckUnsupportedParams checks
13.03.2015 11:04, Peter Krempa пишет: On Thu, Mar 12, 2015 at 18:43:29 +0300, Maxim Nestratov wrote: for memory limits since unset ones are no longer zero Ah, sorry, my bad for not noticing this while reviewing Pavel's series. Never mind. Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index a775348..4c90a18 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1791,10 +1791,10 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def) } if (def-mem.nhugepages || -def-mem.hard_limit || -def-mem.soft_limit || +virMemoryLimitIsSet(def-mem.hard_limit) || +virMemoryLimitIsSet(def-mem.soft_limit) || def-mem.min_guarantee || -def-mem.swap_hard_limit) { +virMemoryLimitIsSet(def-mem.swap_hard_limit)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Memory parameter is not supported ACK, I'll push this once the build with this patch finishes. Peter Thank you -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 4/7] bridge_driver: Drop networkDriverLock() from almost everywhere
On Thu, Mar 12, 2015 at 15:39:18 +0100, Michal Privoznik wrote: Now that we have fine grained locks, there's no need to lock the whole driver. We can rely on self-locking APIs. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/network/bridge_driver.c | 56 - 1 file changed, 5 insertions(+), 51 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 863eeac..bff749a 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v4 4/4] migration: Expose 'cancelling' status to user
On 03/13/2015 06:28 AM, Dr. David Alan Gilbert wrote: It simplifies qemu's job of reporting migration status information (qemu is no longer maintaining one set of states internally and a different set of states externally), and I already have the libvirt counterpart patch ready to go to gracefully accept the new state name. Yes, it does make life simpler in the long run. (It does worry me a bit what happens to new qemu on old libvirt) In the past, we've already stated that it is okay for new qemu to require new libvirt. What is not okay is for new libvirt to require new qemu. This change is okay given those rules. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 08/10] qemu: conf: Add support for memory device cold(un)plug
On 03/04/2015 11:24 AM, Peter Krempa wrote: Add a few helpers that allow to operate with memory device definitions on the domain config and use them to implement memory device coldplug in the qemu driver. --- src/conf/domain_conf.c | 100 +++ src/conf/domain_conf.h | 10 + src/libvirt_private.syms | 4 ++ src/qemu/qemu_driver.c | 15 ++- 4 files changed, 127 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4f20aa6..0f6058b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12687,6 +12687,106 @@ virDomainRNGRemove(virDomainDefPtr def, } +static int +virDomainMemoryFindByDefInternal(virDomainDefPtr def, + virDomainMemoryDefPtr mem, + bool allowAddressFallback) +{ +size_t i; + +for (i = 0; i def-nmems; i++) { +virDomainMemoryDefPtr tmp = def-mems[i]; + +/* address, if present */ +if (allowAddressFallback) { +if (tmp-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) +continue; So this path says - we want address fallback and this device has either DIMM or LAST (oy!) as a type, so go to the next one and ignore this one +} else { +if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE +!virDomainDeviceInfoAddressIsEqual(tmp-info, mem-info)) +continue; This path says - we don't want address fallback and as long as we're DIMM or LAST (oy!), then compare What happens when we add a new address type? It would seem the more straightforward way would be if (type == DIMM !Equal) if (!allowAddressFallback) continue' Otherwise we're falling through to alias, target, etc. checks +} + +/* alias, if present */ +if (mem-info.alias +STRNEQ_NULLABLE(tmp-info.alias, mem-info.alias)) +continue; I thought the NULLABLE checks both elems for NULL... + +/* target info - always present */ +if (tmp-model != mem-model || +tmp-targetNode != mem-targetNode || +tmp-size != mem-size) +continue; + +/* source stuff - match with device */ +if (tmp-pagesize != mem-pagesize) +continue; + +if (!virBitmapEqual(tmp-sourceNodes, mem-sourceNodes)) +continue; + +break; +} + +if (i == def-nmems) +return -1; + +return i; +} + + +int +virDomainMemoryFindByDef(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ +return virDomainMemoryFindByDefInternal(def, mem, false); +} + + +int +virDomainMemoryFindInactiveByDef(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ +int ret; + +if ((ret = virDomainMemoryFindByDefInternal(def, mem, false)) 0) +ret = virDomainMemoryFindByDefInternal(def, mem, true); I would seem Inactive would probably not have the dimm address set, so we're incurring a 2X penalty for perhaps no reason... Unless perhaps the incoming mem def being checked has an address... That is if my incoming has an address, then don't allow fallback; otherwise, well best match. + +return ret; +} + + +int +virDomainMemoryInsert(virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ +int id = def-nmems; + +if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE +virDomainDefHasDeviceAddress(def, mem-info)) { Hmm... so if our incoming mem has an address defined - it could be anything - we're just failing declaring that the domain already contains a device with the same address? Doesn't seem right. And again - we have this problem of TYPE_NONE, TYPE_DIMM, TYPE_LAST and who knows what in the future. +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(Domain already contains a device with the same + address)); +return -1; +} + +if (VIR_APPEND_ELEMENT(def-mems, def-nmems, mem) 0) +return -1; + +return id; +} + + +virDomainMemoryDefPtr +virDomainMemoryRemove(virDomainDefPtr def, + int idx) +{ +virDomainMemoryDefPtr ret = def-mems[idx]; +VIR_DELETE_ELEMENT(def-mems, idx, def-nmems); +return ret; +} + + char * virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 706040d..c38614d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2853,6 +2853,16 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model); typedef const char* (*virEventActionToStringFunc)(int type); typedef int (*virEventActionFromStringFunc)(const char *type);
Re: [libvirt] [PATCHv2.5 06/10] qemu: migration: Forbid migration with memory modules lacking info
On 03/04/2015 11:24 AM, Peter Krempa wrote: Make sure that libvirt has all vital information needed to reliably represent configuration of guest's memory devices in case of a migration. This patch forbids migration in case the required slot number and module base address are not present (failed to be loaded from qemu via monitor). --- src/qemu/qemu_migration.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 20e40aa..a31ce9a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2016,6 +2016,20 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, } } +/* Verify that memory device config can be transferred reliably */ +for (i = 0; i def-nmems; i++) { +virDomainMemoryDefPtr mem = def-mems[i]; + +if (mem-model == VIR_DOMAIN_MEMORY_MODEL_DIMM +mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(domain's dimm info lacks slot ID + or base address)); + +return false; +} +} + return true; } Would this only be possibly true for an offline migration? It would seem an online/live migration that guest startup and continued running of libvirtd would seemingly fill in the values. Then if libvirtd is restarted, the cached copy of the guest with the addresses is read. If the qemuProcessAttach code is implemented, then we have an address. Could this be because we 'ignore' the -2 failures in patch 5? However, if we do, then we've never really added support for this functionality. Of course if the target of the migration does have it, then there's issues. While what's being checked is valid and safely protects us against some unintended mutilation and thus I'd say ACK for just the safety reasons, I'm mostly curious as to the why. Other checks in the API seem to be valid you are not allowed to migrate because we said you couldn't migrate with snapshots, block job running non-USB host devices, or with a specific CPU feature enabled. But, this seems to be - something went wrong and we decided to ignore it, so you cannot migrate this guest. Is there anything we could do to possible fill in the values so that we don't cause failure because of some decision point in libvirt? Of course we couldn't have already gone through this in previous reviews, but my short term memory would have been unplugged ;-) John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 07/10] qemu: add support for memory devices
On 03/04/2015 11:24 AM, Peter Krempa wrote: Add support to start qemu instance with 'pc-dimm' device. Thanks to the refactors we are able to reuse the existing function to determine the parameters. --- src/qemu/qemu_command.c| 130 - src/qemu/qemu_domain.c | 26 - src/qemu/qemu_domain.h | 1 + .../qemuxml2argv-memory-hotplug-dimm.args | 11 ++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c| 1 + 6 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args ACK - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: fix use the wrong type for period
On Fri, Mar 13, 2015 at 05:15:32PM +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1140958 When we set period as unsigned int max value 4294967295 and start the vm, qemu will report error. This becuase we define period as a unsigned int and parse it as a unsigned int, but we use it as a int when set it via QMP in qemuMonitorJSONSetMemoryStatsPeriod, so 4294967295 turn to -1 when we send the QMP command. After check the qemu's code this value type should be int, and found a qemu commit 1f9296b for this values range. Seems no other hypervisor vm use this so i add a check when we parse it. Where to start. NACK to this patch as is. Domains that have INT_MAX period UINT_MAX will disappear after libvirt is restarted and that's not backwards compatible. I couldn't make sense of the commit message, but at least the aim is visible from the code. Anyway, I believe Erik already worked on this issue as I gave him few hints regarding this on one of his patches. Did you talk together about it? It would be a pity if the work was blindly doubled. Looking at this commit message I made a diff to squash in, so I'll post a v2 with it. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 127fc91..54bd5aa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10452,7 +10452,8 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, } ctxt-node = node; -if (virXPathUInt(string(./stats/@period), ctxt, def-period) -1) { +if (virXPathInt(string(./stats/@period), ctxt, def-period) -1 || +def-period 0) { virReportError(VIR_ERR_XML_ERROR, %s, _(invalid statistics collection period)); goto error; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ea463cb..ee0f5fd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1556,7 +1556,7 @@ enum { struct _virDomainMemballoonDef { int model; virDomainDeviceInfo info; -unsigned int period; /* seconds between collections */ +int period; /* seconds between collections */ }; struct _virDomainNVRAMDef { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list pgp86x3oXTMvw.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 05/10] qemu: memdev: Add infrastructure to load memory device information
On 03/04/2015 11:24 AM, Peter Krempa wrote: When using 'dimm' memory devices with qemu, some of the information like the slot number and base address need to be reloaded from qemu after process start so that it reflects the actual state. The state then allows to use memory devices across migrations. --- src/qemu/qemu_domain.c | 49 + src/qemu/qemu_domain.h | 4 ++ src/qemu/qemu_monitor.c | 42 +++ src/qemu/qemu_monitor.h | 14 + src/qemu/qemu_monitor_json.c | 122 +++ src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_process.c | 4 ++ 7 files changed, 240 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4225f38..61b0fc8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2784,6 +2784,55 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, return 0; } + +int +qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int asyncJob) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virHashTablePtr meminfo = NULL; +int rc; +size_t i; + +if (vm-def-nmems == 0) +return 0; + +if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) 0) +return -1; + +rc = qemuMonitorGetMemoryDeviceInfo(priv-mon, meminfo); + +if (qemuDomainObjExitMonitor(driver, vm) 0) +return -1; + +/* if qemu doesn't support the info request, just carry on */ +if (rc == -2) +rc = 0; [1] hmmm Cannot remember if we agreed previously that not having a message was ok (e.g. requires json or requires query-memory-devices command). I guess I figure if we get this far, then some other check has failed us. But, but returning 0 we say - yep it worked, which of course isn't true. I'm conflicted since future patches become affected... + +if (rc 0) +return -1; + +for (i = 0; i vm-def-nmems; i++) { [1] If the real rc == -2, then vm-def-nmems 0 and we enter this loop which is probably not a good idea. +virDomainMemoryDefPtr mem = vm-def-mems[i]; +qemuMonitorMemoryDeviceInfoPtr dimm; + +if (!mem-info.alias) +continue; + +if (!(dimm = virHashLookup(meminfo, mem-info.alias))) +continue; + +mem-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM; +mem-info.addr.dimm.slot = dimm-slot; +mem-info.addr.dimm.base = dimm-address; +} + +virHashFree(meminfo); +return 0; +} + + bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, virDomainDefPtr src, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9760095..b2be323 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -391,6 +391,10 @@ extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig; int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob); +int qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int asyncJob); + bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, virDomainDefPtr src, virDomainDefPtr dst); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 94495cd..34673e1 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4359,3 +4359,45 @@ void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread) VIR_FREE(iothread-name); VIR_FREE(iothread); } + + +/** + * qemuMonitorGetMemoryDeviceInfo: + * @mon: pointer to the monitor + * @info: Location to return the hash of qemuMonitorMemoryDeviceInfo + * + * Retrieve state and addresses of frontend memory devices present in + * the guest. + * + * Returns 0 on success and fills @info with a newly allocated struct; if the + * data can't be retrieved due to lack of support in qemu, returns -2. On + * other errors returns -1. + */ +int +qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon, + virHashTablePtr *info) +{ +VIR_DEBUG(mon=%p info=%p, mon, info); +int ret; + +*info = NULL; + +if (!mon) { +virReportError(VIR_ERR_INVALID_ARG, %s, + _(monitor must not be NULL)); +return -1; +} + +if (!mon-json) +return -2; + +if (!(*info = virHashCreate(10, virHashValueFree))) +return -1; + +if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, *info)) 0) { +virHashFree(*info); +*info = NULL; +} + +return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index
Re: [libvirt] [Qemu-devel] [PATCH v4 4/4] migration: Expose 'cancelling' status to user
* Eric Blake (ebl...@redhat.com) wrote: On 03/13/2015 04:49 AM, Paolo Bonzini wrote: On 09/03/2015 07:45, zhanghailiang wrote: 'cancelling' status was introduced by commit 51cf4c1a, mainly to avoid a possible start of a new migration process while the previous one still exists. But we didn't expose this status to user, instead we returned the 'active' state. Here, we expose it to the user (such as libvirt), 'cancelling' status only occurs for a short window before the migration aborts, so for users, if they cancel a migration process, it will observe 'cancelling' status occasionally. Testing revealed that with older libvirt (anything 1.2.13 or less) will print an odd error message if the state is seen, but that the migration is still properly cancelled. Newer libvirt will be patched to recognize the new state without the odd error message. Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com Reviewed-by: Eric Blake ebl...@redhat.com Cc: libvir-list@redhat.com Why is this necessary? It simplifies qemu's job of reporting migration status information (qemu is no longer maintaining one set of states internally and a different set of states externally), and I already have the libvirt counterpart patch ready to go to gracefully accept the new state name. Yes, it does make life simpler in the long run. (It does worry me a bit what happens to new qemu on old libvirt) Dave -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] Add test for virtio serial port assignment
On Tue, Mar 03, 2015 at 15:44:26 +0100, Ján Tomko wrote: Add a test to demonstrate the effect of this series. --- .../qemuxml2argv-channel-virtio-autoassign.args| 20 + .../qemuxml2argv-channel-virtio-autoassign.xml | 50 ++ tests/qemuxml2argvtest.c | 2 + 3 files changed, 72 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: read backing chain names from qemu
On 03/13/2015 02:02 AM, Peter Krempa wrote: @@ -16172,8 +16169,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, } qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath, - speed, mode, async); +if (baseSource) +basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src, I remember that at some point accessing of domain definition while in the monitor was not okay for some reason, but I can't now remember why nor whether it was fixed. Oh, right. You're thinking of CVE-2013-6458. That problem was that as soon as we enter the monitor, we drop locks. If we do not already own a block job, then some other parallel API could be hot-unplugging a disk before we regain control, freeing 'disk' before we dereference it. But we fixed that problem by guaranteeing that we always own the job early enough (no other thread can hot-unplug the disk as long as we own the job), so it is not an issue for this patch. - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. Shouldn't we employ something as in gnulib, where copyrights would be bumped at once everywhere? Might be nice, but one wrinkle. Gnulib has a single copyright holder (FSF), so they can afford to bump all files at once (the bump is also owned by FSF, so FSF adding another year to its copyright is appropriate). But libvirt is split among multiple copyright holders - Red Hat can't claim copyright over all files, so it wouldn't be wise to bump all files, just the ones that Red Hat has already touched. Personally, I've just got an emacs hook that checks if any file I touch has an up-to-date copyright line. +static char * +qemuMonitorJSONDiskNameLookupOne(virJSONValuePtr image, + virStorageSourcePtr top, + virStorageSourcePtr target) +{ +virJSONValuePtr backing; +char *ret; + +if (!top) +return NULL; In case the backing chain as remembered by libvirt is shorter than what qemu sees you don't report error. Since the caller checks whether an error was set and if not then adds one, please state this fact in a comment here as it's not obvious until you follow the call chain. Will do. +if (top != target) { +backing = virJSONValueObjectGet(image, backing-image); +return qemuMonitorJSONDiskNameLookupOne(backing, top-backingStore, +target); Also the recursion doesn't take into account that for some reason qemu might report a shorter chain than libvirt thinks, which would crash here. Oh, good catch (and looks like it explains what Shanzhi reported). +if (!dev || dev-type != VIR_JSON_TYPE_OBJECT) { [1] +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(block info device entry was not in expected format)); +goto cleanup; +} + +if ((thisdev = virJSONValueObjectGetString(dev, device)) == NULL) { You are mixing styles of cheching of the pointer to be non-null within a few lines ([1]) Copy-and-paste from another recursive parser of query-block information, but I can make it more consistent. ACK if you add the comment and fix the potential crash. I'm currently OK with accessing domain definition while it's unlocked (but guarded via the domain job) as I don't have an counter example where it wouldn't work correctly. I'm still a bit worried by Shanzhi's report of a crash; maybe I still have a race condition. That is, we change libvirt's notion of the chain length after a commit based on response to a qemu event rather than a user command - I was thinking that libvirt's chain and qemu's chain will always be the same length, but since Shanzhi provided a stack trace where it is not true, I'm wondering if the qemu chain being shorter than the libvirt chain might mean that we have some sort of window where a qemu event happens at the wrong moment when repeatedly hammering on consecutive commits. So I'll post a v3 after more testing rather than just blindly going on this ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v4 4/4] migration: Expose 'cancelling' status to user
On 03/13/2015 04:49 AM, Paolo Bonzini wrote: On 09/03/2015 07:45, zhanghailiang wrote: 'cancelling' status was introduced by commit 51cf4c1a, mainly to avoid a possible start of a new migration process while the previous one still exists. But we didn't expose this status to user, instead we returned the 'active' state. Here, we expose it to the user (such as libvirt), 'cancelling' status only occurs for a short window before the migration aborts, so for users, if they cancel a migration process, it will observe 'cancelling' status occasionally. Testing revealed that with older libvirt (anything 1.2.13 or less) will print an odd error message if the state is seen, but that the migration is still properly cancelled. Newer libvirt will be patched to recognize the new state without the odd error message. Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com Reviewed-by: Eric Blake ebl...@redhat.com Cc: libvir-list@redhat.com Why is this necessary? It simplifies qemu's job of reporting migration status information (qemu is no longer maintaining one set of states internally and a different set of states externally), and I already have the libvirt counterpart patch ready to go to gracefully accept the new state name. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: read backing chain names from qemu
On Fri, Mar 13, 2015 at 07:01:06AM -0600, Eric Blake wrote: On 03/13/2015 02:02 AM, Peter Krempa wrote: @@ -16172,8 +16169,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, } qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath, - speed, mode, async); +if (baseSource) +basePath = qemuMonitorDiskNameLookup(priv-mon, device, disk-src, I remember that at some point accessing of domain definition while in the monitor was not okay for some reason, but I can't now remember why nor whether it was fixed. Oh, right. You're thinking of CVE-2013-6458. That problem was that as soon as we enter the monitor, we drop locks. If we do not already own a block job, then some other parallel API could be hot-unplugging a disk before we regain control, freeing 'disk' before we dereference it. But we fixed that problem by guaranteeing that we always own the job early enough (no other thread can hot-unplug the disk as long as we own the job), so it is not an issue for this patch. - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. Shouldn't we employ something as in gnulib, where copyrights would be bumped at once everywhere? Might be nice, but one wrinkle. Gnulib has a single copyright holder (FSF), so they can afford to bump all files at once (the bump is also owned by FSF, so FSF adding another year to its copyright is appropriate). But libvirt is split among multiple copyright holders - Red Hat can't claim copyright over all files, so it wouldn't be wise to bump all files, just the ones that Red Hat has already touched. Personally, I've just got an emacs hook that checks if any file I touch has an up-to-date copyright line. Technically there is no need to actually assert copyright over the code at all, since copyright is an automatic right you get the moment you author the code. Given that the copyright notice is not even required in the first place, asserting a year alongside the copyright notice is by implication not required either, nor is updating the year when you change code. Adding the Copyright lines is at most an informative step, to assist those reading the code in seeing its providence ownership. Of course GIT history is much more useful for that purpose, but not everyone will receive a copy of GIT repo when they receive the code. In essence, the Copyright lines had a moderate benefit in clarifying ownership, but no legal benefit. By all means include a date when first starting a new file, but I think updating existing dates is pretty much a waste of time. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] parallels: minor cleanup
indentation is fixed, unnecessary error message removed, unnecessary job freeing removed Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c |5 + 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 1025da5..f6350df 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -698,7 +698,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) goto cleanup; pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, net-ifname, buflen); -prlsdkCheckRetGoto(pret, cleanup); +prlsdkCheckRetGoto(pret, cleanup); pret = PrlVmDev_GetIndex(netAdapter, netAdapterIndex); prlsdkCheckRetGoto(pret, cleanup); @@ -1360,7 +1360,6 @@ prlsdkLoadDomains(parallelsConnPtr privconn) error: PrlHandle_Free(result); -PrlHandle_Free(job); return -1; } @@ -1740,8 +1739,6 @@ prlsdkDomainChangeState(virDomainPtr domain, pdom = dom-privateData; pret = chstate(privconn, pdom-sdkdom); -virReportError(VIR_ERR_OPERATION_FAILED, - _(Can't change domain state: %d), pret); if (PRL_FAILED(pret)) { virResetLastError(); -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] parallels: set cpu mode when applying xml configuration
From: Mikhail Feoktistov mfeoktis...@parallels.com Otherwise exporting existing domain config and defining a new one like this: virsh -c parallels:///system dumpxml instance01 my.xml virsh -c parallels:///system define my.xml leads to an error because PCS default x64 mode turns to x32. Thus, we need to set correct cpuMode in prlsdkDoApplyConfig() explicitly. Signed-off-by: Mikhail Feoktistov mfeoktis...@parallels.com Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c | 13 + 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 4c90a18..b6026fd 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2957,6 +2957,19 @@ prlsdkDoApplyConfig(virConnectPtr conn, prlsdkCheckRetGoto(pret, error); VIR_FREE(mask); +switch (def-os.arch) { +case VIR_ARCH_X86_64: +pret = PrlVmCfg_SetCpuMode(sdkdom, PCM_CPU_MODE_64); +break; +case VIR_ARCH_I686: +pret = PrlVmCfg_SetCpuMode(sdkdom, PCM_CPU_MODE_32); +break; +default: +virReportError(VIR_ERR_INTERNAL_ERROR, _(Unknown CPU mode: %X), def-os.arch); +goto error; +} +prlsdkCheckRetGoto(pret, error); + if (prlsdkClearDevices(sdkdom) 0) goto error; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/4] parallels: fixes and cleanups
From: Maxim Nestratov mnestra...@parallels.com v2 change: - rebased Maxim Nestratov (3): parallels: don't forget to unlock domain if unregister fails parallels: fix home directory for VMs parallels: minor cleanup Mikhail Feoktistov (1): parallels: set cpu mode when applying xml configuration -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] parallels: don't forget to unlock domain if unregister fails
Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_driver.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index d2907cf..aeb43ad 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -948,6 +948,7 @@ parallelsDomainUndefineFlags(virDomainPtr domain, { parallelsConnPtr privconn = domain-conn-privateData; virDomainObjPtr dom = NULL; +int ret; virCheckFlags(0, -1); @@ -957,7 +958,11 @@ parallelsDomainUndefineFlags(virDomainPtr domain, return -1; } -return prlsdkUnregisterDomain(privconn, dom); +ret = prlsdkUnregisterDomain(privconn, dom); +if (ret) + virObjectUnlock(dom); + +return ret; } static int -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/7] Drop network driver lock
On 13.03.2015 11:07, Peter Krempa wrote: On Thu, Mar 12, 2015 at 15:39:14 +0100, Michal Privoznik wrote: Hopefully, the last version. Again, some patches are ACKed already, but I'm sending them again. Not to trash the review bandwidth, but for reviewer to get better picture. Patches 5-7 are already ACKed. Peter Thanks a lot! I've pushed this. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] parallels: fix home directory for VMs
Failures of parallelsStorageOpen occured because we incorrectly treated path to VM' configuration file as a directory. Now initialization of parallels VM domains home directory is fixed. Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index b6026fd..1025da5 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1255,6 +1255,14 @@ prlsdkLoadDomain(parallelsConnPtr privconn, pret = PrlVmCfg_GetHomePath(sdkdom, pdom-home, buflen); prlsdkCheckRetGoto(pret, error); +/* For VMs pdom-home is actually /directory/config.pvs */ +if (!IS_CT(def)) { +/* Get rid of /config.pvs in path string */ +char *s = strrchr(pdom-home, '/'); +if (s) +*s = '\0'; +} + if (olddom) { /* assign new virDomainDef without any checks */ /* we can't use virDomainObjAssignDef, because it checks -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Applying for GSoC project 'Introducing job control to the storage driver' in libvirt
Thank you, I will take a look on those APIs you suggested. BTW, about the project virsh, I found libvirt-client(http://www.rpmfind.net/linux/rpm2html/search.php?query=libvirt-client) already support automatic completion on virsh commands. It is a big progress on the project. So, why it hasn't merged into git source repository? 2015-03-10 23:58 GMT+08:00 Michal Privoznik mpriv...@redhat.com: On 09.03.2015 07:29, Taowei Luo wrote: Hello, everyone. Hey! I'm Taowei Luo. A participator in Google Summer of Code last year for the project rewriting vbox driver in libvirt. I am so glad that my project isn't found in this year's GSoC project list :) Having a nice experience last year, libvirt is my top option to join the GSoC 2015. And my interesting project is “Introducing job control to the storage driver”. It's nice to see people returning. I have read some materials about this project. Including I noticed that Tucker had attempted it last year. It seems he didn't make it through the final evaluation. During his working, some discussion were made in mail-list. It is really inspiring to me. So I have some basic ideas for now. The project requires job control functions (at least, reporting the progress) in storage driver. Obviously, it contains two part. First find codes that really do the storage work which may take a long period and can be asynchronized. Then, extract it to the job control part so make it under the control. It would not be hard to find codes that really need asynchronization. Maybe by dumping the debug message and tracing the function calls. So the big part is design the APIs for job control. I think the goal for API design is not only make it workable but also make it reasonable and extendable. The idea is to turn qemuDomainObj*Job* a separate module, that would work over virObject. Subsequently, we can add BeginJob and EndJob APIs into storage driver too. Moreover, places where libvirt monitors progress of a storage operation, we can fill in jobInfo strucut (although, that one in qemu is not generic enough I guess, so maybe we need to invent a new one). Then, CancelJob API can be implemented in storage driver too. To achieve this, a lot of details were discussed in last year's proceeding. I summered it and add my own opinion. Parallel jobs: The idea result is that several jobs work in parallel and libvirt monitors and controls it. There are two ways for parallel: thread and process. I prefer process. In process, we can easier implement the idea of *control* by signal. Process has better independence than thread. What's more, it is a low coupling design. Libvirt already uses that. virCommandExec* is to be find all over the storage driver. Synchronization: process can use system level lock to make sure it obtain the resources. If the process can't obtain it could exit with failure (or wait). In process, we can leave most resource competition handling by OS. If thread is used instead, we need to think about resource competition between libvirt and other process, and at the same time, those competitions in libvirt thread. Management: We execute those asynchronous codes in a new process. In libvirt, it invokes those processes with parsed arguments. Libvirtd would have a process pool to store the pids and some attached information for each process. Signal would be used to communicate between process and libvirt. Expandability: Some other jobs like domain migration could be in implemented under this design. It's all about creating new jobs with parsed arguments, which tells the child process what to do and what resources they need. Privacy: If new jobs are created in process, user may access the process directly and not noticing libvirt. Function sigaction(), which provides the pid of sending process, could be used to register responding functions. Meanwhile, atexit() register functions that execute when the process is going to exit. It is helpful on notifying libvirtd the end of the job. This is already handled by virCommand subsystem. What already have: Besides the discussion, some other resources would be helpful for this project. qemu has a prototype for job control in migration. We already have gnulib and tools in util/virprocess.h and util/virthread.h to achieve parallel. When undertaking, I would firstly implement the job control part. It would have some basic functions. Make one storage API work in it. Then, adding job control support for the rest APIs. This is all about what I came up with. Maybe those ideas are old and repeating. But I think it is a workable plan. Waiting for feedback. Majority looks reasonable to me. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] Add functions to track virtio-serial addresses
On Tue, Mar 03, 2015 at 15:44:27 +0100, Ján Tomko wrote: Store the available ports of a virtio-serial controller in a virBitmap. The bitmaps are stored in a hash table - the controller index formatted as a string. Buses are not tracked, because they aren't supported by QEMU. --- src/conf/domain_addr.c | 382 +++ src/conf/domain_addr.h | 45 ++ src/libvirt_private.syms | 8 + 3 files changed, 435 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index fb4a76f..654c95a 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c +int ... +virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr addrs, + virDomainDefPtr def) +{ +size_t i; + +for (i = 0; i def-ncontrollers; i++) { +if (virDomainVirtioSerialAddrSetAddController(addrs, + def-controllers[i]) 0) +return -1; +} + +return 0; +} + +void +virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs) +{ +if (addrs) { +virHashFree(addrs-used); +VIR_FREE(addrs); +} +} + +/* + * Eww, this function compares two unsigned integers stored as a string + */ Okay, technically that works as expected, but is it really necessary? If the user doesn't specify the serial bus to connect the port to, we can IMO attach it to any of the available ones. For that approach you can use the virHashSearch function combined with virBitmapNextClearBit. Otherwise it would be better to just use an array and not bother with a bitmap. +static int +virDomainVirtioSerialAddrCompare(const virHashKeyValuePair *a, + const virHashKeyValuePair *b) +{ +const char *key_a = a-key; +const char *key_b = b-key; + +size_t len_a = strlen(key_a); +size_t len_b = strlen(key_b); + +/* with no padding/negative numbers allowed, the longer string + * contains a larger number */ +if (len_a len_b) +return -1; +else if (len_a len_b) +return 1; +else +return strncmp(key_a, key_b, len_a); +} + +static int +virDomainVirtioSerialAddrNext(virDomainVirtioSerialAddrSetPtr addrs, + virDomainDeviceVirtioSerialAddress *addr, + bool allowZero) +{ +virBitmapPtr cur = NULL; +char *str = NULL; +int ret = -1; +virHashKeyValuePairPtr arr = NULL; +size_t i, ncontrollers; +size_t curidx; +ssize_t port, start = 0; +unsigned int controller; + +/* port number 0 is reserved for virtconsoles */ +if (allowZero) +start = -1; + +/* What controller was the last assigned address on? */ +if (virAsprintf(str, %u, addrs-next.controller) 0) +goto cleanup; Also making sure that the last controller is always used doens't make that much sense IMO. + +if (!(cur = virHashLookup(addrs-used, str))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(The last used virtio serial controller is missing + from the address set hash table)); +goto cleanup; +} Nor finding it problematic if it's missing. + +/* Look for a free port on the current controller */ +if ((port = virBitmapNextClearBit(cur, start + addrs-next.port)) = 0) { +controller = addrs-next.controller; +goto success; +} + +ncontrollers = virHashSize(addrs-used); +arr = virHashGetItems(addrs-used, virDomainVirtioSerialAddrCompare); +if (!arr) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Unable to get the hash table as an array)); +goto cleanup; +} + +/* Find its position in the hash array */ +for (i = 0; i ncontrollers; i++) { +if (arr[i].value == cur) { +curidx = i; +break; +} +} +if (i == ncontrollers) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(The last used virtio serial controller is missing from the set)); +goto cleanup; +} + +/* Search for a free port after the current controller */ +for (i = curidx; i ncontrollers; i++) { +cur = (virBitmapPtr) arr[i].value; +if ((port = virBitmapNextClearBit(cur, start)) = 0) { +if (virStrToLong_ui(arr[i].key, NULL, 10, controller) 0) +goto cleanup; +goto success; +} +} + +for (i = 0; i curidx; i++) { +cur = (virBitmapPtr) arr[i].value; +if ((port = virBitmapNextClearBit(cur, start)) = 0) { +if (virStrToLong_ui(arr[i].key, NULL, 10, controller) 0) +goto cleanup; +goto success; +} +} Or doing any of this.
Re: [libvirt] Applying for GSoC project 'Introducing job control to the storage driver' in libvirt
On 13.03.2015 15:00, Taowei Luo wrote: Thank you, I will take a look on those APIs you suggested. BTW, about the project virsh, I found libvirt-client(http://www.rpmfind.net/linux/rpm2html/search.php?query=libvirt-client) already support automatic completion on virsh commands. It is a big progress on the project. So, why it hasn't merged into git source repository? libvirt-client *is* virsh. I mean, it's a package containing virsh. And the package is built from sources in git repo. So the problem I'm describing stays the same. Long story short, on this input: virsh # startTABTAB I want virsh to offer me a list of inactive domains, and on this input: virsh # net-destroy defTABTAB to autocomplete 'default' (assuming there's no other active network with prefix 'def'). Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] parallels: switch off offline management feature
which is on by default when a new VM/CT is created. We should do this because this feature can't be controlled by libvirt now and it sets up some iptables rules. So it's better to do this to avoid potential conflict of different set of rules or to avoid unexpected behavior. Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 748a308..a0a2ba0 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -3062,6 +3062,9 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) pret = PrlVmCfg_SetDefaultConfig(sdkdom, srvconf, PVS_GUEST_VER_LIN_REDHAT, 0); prlsdkCheckRetGoto(pret, cleanup); +pret = PrlVmCfg_SetOfflineManagementEnabled(sdkdom, 0); +prlsdkCheckRetGoto(pret, cleanup); + ret = prlsdkDoApplyConfig(conn, sdkdom, def); if (ret) goto cleanup; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] parallels: set network adapter device status to connected
when a new network adapter device is added Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index f581fbb..9588163 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2653,7 +2653,7 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, virDomainN pret = PrlVmDev_SetEnabled(sdknet, 1); prlsdkCheckRetGoto(pret, cleanup); -pret = PrlVmDev_SetConnected(sdknet, net-linkstate); +pret = PrlVmDev_SetConnected(sdknet, 1); prlsdkCheckRetGoto(pret, cleanup); if (net-ifname) { -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] parallels: introduce and use string constants for network types and names
Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_network.c |6 +++--- src/parallels/parallels_sdk.c |6 +++--- src/parallels/parallels_utils.h |8 +++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index 1d3b694..bb7ec5e 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -211,12 +211,12 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; } -if (STREQ(tmp, bridged)) { +if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) { def-forward.type = VIR_NETWORK_FORWARD_BRIDGE; if (parallelsGetBridgedNetInfo(def, jobj) 0) goto cleanup; -} else if (STREQ(tmp, host-only)) { +} else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) { def-forward.type = VIR_NETWORK_FORWARD_NONE; if (parallelsGetHostOnlyNetInfo(def, def-name) 0) @@ -248,7 +248,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn) def-forward.type = VIR_NETWORK_FORWARD_ROUTE; -if (VIR_STRDUP(def-name, PARALLELS_ROUTED_NETWORK_NAME) 0) +if (VIR_STRDUP(def-name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) 0) goto cleanup; if (virUUIDParse(PARALLELS_ROUTED_NETWORK_UUID, def-uuid) 0) { diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 89a9a58..76cbb02 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -708,7 +708,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) * always up */ net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP; if (VIR_STRDUP(net-data.network.name, - PARALLELS_ROUTED_NETWORK_NAME) 0) + PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) 0) goto cleanup; return 0; } @@ -727,7 +727,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) if (emulatedType == PNA_ROUTED) { if (VIR_STRDUP(net-data.network.name, - PARALLELS_ROUTED_NETWORK_NAME) 0) + PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) 0) goto cleanup; } else { pret = PrlVmDevNet_GetVirtualNetworkId(netAdapter, NULL, buflen); @@ -2653,8 +2653,8 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net) pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup); -if (STREQ(net-data.network.name, PARALLELS_ROUTED_NETWORK_NAME)) { pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED); +if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { prlsdkCheckRetGoto(pret, cleanup); } else { pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net-data.network.name); diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h index 394548a..0f29374 100644 --- a/src/parallels/parallels_utils.h +++ b/src/parallels/parallels_utils.h @@ -47,7 +47,13 @@ _(no domain with matching uuid '%s'), uuidstr); \ } while (0) -# define PARALLELS_ROUTED_NETWORK_NAME Routed +# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME Routed +# define PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME Bridged + +# define PARALLELS_REQUIRED_HOSTONLY_NETWORK Host-Only +# define PARALLELS_HOSTONLY_NETWORK_TYPE host-only +# define PARALLELS_REQUIRED_BRIDGED_NETWORK Bridged +# define PARALLELS_BRIDGED_NETWORK_TYPE bridged struct _parallelsConn { virMutex lock; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/7] parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE
network adapter is used --- src/parallels/parallels_sdk.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index a0a2ba0..4c90a18 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2246,7 +2246,8 @@ static int prlsdkCheckSerialUnsupportedParams(virDomainChrDefPtr chr) static int prlsdkCheckNetUnsupportedParams(virDomainNetDefPtr net) { -if (net-type != VIR_DOMAIN_NET_TYPE_NETWORK) { +if (net-type != VIR_DOMAIN_NET_TYPE_NETWORK +net-type != VIR_DOMAIN_NET_TYPE_BRIDGE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Specified network adapter type is not supported by Parallels Cloud Server.)); -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/4] qemu: Don't duplicate errors when settings stats period
In order not to leave old error messages set, this patch refactors the code so the error is reported only when acted upon. The only such place already rewrites any error, so cleaning up all the error reporting in qemuMonitorSetMemoryStatsPeriod() is enough. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_monitor.c | 31 ++- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d869a72..18f866f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1,7 +1,7 @@ /* * qemu_monitor.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1709,27 +1709,40 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, return ret; } +/** + * qemuMonitorSetMemoryStatsPeriod: + * + * This function sets balloon stats update period. + * + * Returns 0 on success and -1 on error, but does *not* set an error. + */ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon, int period) { int ret = -1; VIR_DEBUG(mon=%p period=%d, mon, period); -if (!mon) { -virReportError(VIR_ERR_INVALID_ARG, %s, - _(monitor must not be NULL)); +if (!mon) return -1; -} -if (!mon-json) { -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, - _(JSON monitor is required)); +if (!mon-json) +return -1; + +if (period 0) return -1; -} if (qemuMonitorFindBalloonObjectPath(mon, /) == 0) { ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon-balloonpath, period); + +/* + * Most of the calls to this function are supposed to be + * non-fatal and the only one that should be fatal wants its + * own error message. More details for debugging will be in + * the log file. + */ +if (ret 0) +virResetLastError(); } mon-ballooninit = true; return ret; -- 2.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/4] Fix errors with memory balloon stats period
Nothing big, just some cleanup and then the fix in last patch. More info in particular commit messages. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140958 v1 is here: https://www.redhat.com/archives/libvir-list/2015-March/msg00665.html Martin Kletzander (4): util: Make sure the comment about virBufferAddBuffer is true conf: Reorder elements inside memballoon qemu: Don't duplicate errors when settings stats period conf: Use correct type for balloon stats period docs/formatdomain.html.in | 2 ++ docs/schemas/domaincommon.rng | 28 src/conf/domain_conf.c | 37 -- src/conf/domain_conf.h | 2 +- src/qemu/qemu_monitor.c| 31 -- src/qemu/qemu_process.c| 2 +- src/util/virbuffer.c | 14 +--- .../qemuxml2xmlout-balloon-device-period.xml | 30 ++ tests/qemuxml2xmltest.c| 1 + 9 files changed, 101 insertions(+), 46 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-period.xml -- 2.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] util: Make sure the comment about virBufferAddBuffer is true
Change it so it really *always* eats the @toadd buffer. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/util/virbuffer.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 96a0f16..0089d1b 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -1,7 +1,7 @@ /* * virbuffer.c: buffers for libvirt * - * Copyright (C) 2005-2008, 2010-2014 Red Hat, Inc. + * Copyright (C) 2005-2008, 2010-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -188,23 +188,27 @@ virBufferAddBuffer(virBufferPtr buf, virBufferPtr toadd) { unsigned int needSize; -if (!buf || !toadd) +if (!toadd) return; +if (!buf) +goto done; + if (buf-error || toadd-error) { if (!buf-error) buf-error = toadd-error; -virBufferFreeAndReset(toadd); -return; +goto done; } needSize = buf-use + toadd-use; if (virBufferGrow(buf, needSize - buf-use) 0) -return; +goto done; memcpy(buf-content[buf-use], toadd-content, toadd-use); buf-use += toadd-use; buf-content[buf-use] = '\0'; + + done: virBufferFreeAndReset(toadd); } -- 2.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] parallels: better bridge network interface support
In order to support 'bridge' network adapters in parallels driver we need to plug our veth devices into corresponding linux bridges. We are going to do this by reusing our abstraction of Virtual Networks in terms of PCS. On a domain creation, we create a new Virtual Network naming it with the same name as a source bridge for each network interface. Having done this, we plug PCS veth interfaces created with names of target dev into specified bridges using our standard PCS procedures Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c | 99 +++- 1 files changed, 86 insertions(+), 13 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 76cbb02..f581fbb 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -691,9 +691,6 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) /* use device name, shown by prlctl as target device * for identifying network adapter in virDomainDefineXML */ -pret = PrlVmDev_GetIndex(netAdapter, netAdapterIndex); -prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, NULL, buflen); prlsdkCheckRetGoto(pret, cleanup); @@ -703,6 +700,9 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, net-ifname, buflen); prlsdkCheckRetGoto(pret, cleanup); +pret = PrlVmDev_GetIndex(netAdapter, netAdapterIndex); +prlsdkCheckRetGoto(pret, cleanup); + if (isCt netAdapterIndex == (PRL_UINT32) -1) { /* venet devices don't have mac address and * always up */ @@ -740,6 +740,16 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) net-data.network.name, buflen); prlsdkCheckRetGoto(pret, cleanup); + +/* + * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters + * except those whose Virtual Network Id differ from Parallels + * predefined ones such as PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME + * and PARALLELS_DONAIN_ROUTED_NETWORK_NAME + */ +if (!STREQ(net-data.network.name, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) +net-type = VIR_DOMAIN_NET_TYPE_BRIDGE; + } pret = PrlVmDev_IsConnected(netAdapter, isConnected); @@ -2625,10 +2635,12 @@ static const char * prlsdkFormatMac(virMacAddrPtr mac, char *macstr) return macstr; } -static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net) +static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, virDomainNetDefPtr net) { PRL_RESULT pret; PRL_HANDLE sdknet = PRL_INVALID_HANDLE; +PRL_HANDLE vnet = PRL_INVALID_HANDLE; +PRL_HANDLE job = PRL_INVALID_HANDLE; int ret = -1; char macstr[PRL_MAC_STRING_BUFNAME]; @@ -2653,10 +2665,39 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net) pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup); -pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED); -if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { +if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) { +if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { +pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED); +prlsdkCheckRetGoto(pret, cleanup); +} else if (STREQ(net-data.network.name, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) { +pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net-data.network.name); +prlsdkCheckRetGoto(pret, cleanup); +} +} else if (net-type == VIR_DOMAIN_NET_TYPE_BRIDGE) { +/* + * For this type of adapter we create a new + * Virtual Network assuming that bridge with given name exists + * Failing creating this means domain creation failure + */ +pret = PrlVirtNet_Create(vnet); prlsdkCheckRetGoto(pret, cleanup); -} else { + +pret = PrlVirtNet_SetNetworkId(vnet, net-data.network.name); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVirtNet_SetNetworkType(vnet, PVN_BRIDGED_ETHERNET); +prlsdkCheckRetGoto(pret, cleanup); + +job = PrlSrv_AddVirtualNetwork(privconn-server, vnet, 0); +if (PRL_FAILED(pret = waitJob(job, privconn-jobTimeout))) +goto cleanup; + +pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET); +prlsdkCheckRetGoto(pret, cleanup); + pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net-data.network.name); prlsdkCheckRetGoto(pret, cleanup); } @@ -2669,10
[libvirt] [PATCH v2 0/7] bridge network support enhancement and other network fixes
From: Maxim Nestratov mnestra...@parallels.com v2 change: - rebased on recent network rework Maxim Nestratov (7): parallels: introduce and use string constants for network types and names parallels: fix parallelsLoadNetworks parallels: better bridge network interface support parallels: set network adapter device status to connected parallels: make E1000 network adapter type default parallels: switch off offline management feature parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] parallels: fix parallelsLoadNetworks
Don't fail initialization of parallels driver if parallelsLoadNetwork fails for optional networks. This can happen when some of them are added manually and configured incompletely. PCS requires only two networks created automatically (named Host-Only and Bridged), others are optional and their incompletenes can be ignored. Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_network.c | 43 +++- 1 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index bb7ec5e..4dc7115 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -180,9 +180,10 @@ static int parallelsGetHostOnlyNetInfo(virNetworkDefPtr def, const char *name) return ret; } -static virNetworkObjPtr +static int parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) { +int ret = -1; virNetworkObjPtr net; virNetworkDefPtr def; const char *tmp; @@ -214,13 +215,25 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) { def-forward.type = VIR_NETWORK_FORWARD_BRIDGE; -if (parallelsGetBridgedNetInfo(def, jobj) 0) +if (parallelsGetBridgedNetInfo(def, jobj) 0) { + +/* Only mandatory networks required to be configured completely */ +if (!STREQ(def-name, PARALLELS_REQUIRED_BRIDGED_NETWORK)) +ret = 0; + goto cleanup; +} } else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) { def-forward.type = VIR_NETWORK_FORWARD_NONE; -if (parallelsGetHostOnlyNetInfo(def, def-name) 0) +if (parallelsGetHostOnlyNetInfo(def, def-name) 0) { + +/* Only mandatory networks required to be configured completely */ +if (!STREQ(def-name, PARALLELS_REQUIRED_HOSTONLY_NETWORK)) +ret = 0; + goto cleanup; +} } else { parallelsParseError(); goto cleanup; @@ -230,14 +243,16 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; net-active = 1; net-autostart = 1; -return net; +virNetworkObjEndAPI(net); +ret = 0; +return ret; cleanup: virNetworkDefFree(def); -return NULL; +return ret; } -static virNetworkObjPtr +static int parallelsAddRoutedNetwork(parallelsConnPtr privconn) { virNetworkObjPtr net = NULL; @@ -264,18 +279,18 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn) } net-active = 1; net-autostart = 1; +virNetworkObjEndAPI(net); -return net; +return 0; cleanup: virNetworkDefFree(def); -return NULL; +return -1; } static int parallelsLoadNetworks(parallelsConnPtr privconn) { virJSONValuePtr jobj, jobj2; -virNetworkObjPtr net = NULL; int ret = -1; int count; size_t i; @@ -300,22 +315,18 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn) goto cleanup; } -net = parallelsLoadNetwork(privconn, jobj2); -if (!net) +ret = parallelsLoadNetwork(privconn, jobj2); +if (!ret) goto cleanup; -else -virNetworkObjEndAPI(net); - } -if (!(net = parallelsAddRoutedNetwork(privconn))) +if (!(ret = parallelsAddRoutedNetwork(privconn))) goto cleanup; ret = 0; cleanup: virJSONValueFree(jobj); -virNetworkObjEndAPI(net); return ret; } -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] parallels: make E1000 network adapter type default
Signed-off-by: Maxim Nestratov mnestra...@parallels.com --- src/parallels/parallels_sdk.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 9588163..748a308 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2665,6 +2665,10 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, virDomainN pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup); +/* Other alternatives: PNT_VIRTIO, PNT_RTL */ +pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); +prlsdkCheckRetGoto(pret, cleanup); + if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) { if (STREQ(net-data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED); -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: fix report of non-active commit completion
Commit f182da20 (v1.2.6) caused a slight regression in virsh reporting of a non-active block job; where it used to state Commit complete, it now states Now in synchronized phase. But the synchronized phase is only possible for an active commit. For a reproducer, I created a chain 'a - b - c - d - e' and ran virsh blockcommit $dom vda --top c --base a --verbose --wait * tools/virsh-domain.c (cmdBlockCommit): Synchronized phase is only possible on active commits. Signed-off-by: Eric Blake ebl...@redhat.com --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1a364bb..b4e9cb0 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2018,7 +2018,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, \n%s, _(Commit aborted)); else if (pivot) vshPrint(ctl, \n%s, _(Successfully pivoted)); -else if (!finish) +else if (!finish active) vshPrint(ctl, \n%s, _(Now in synchronized phase)); else vshPrint(ctl, \n%s, _(Commit complete)); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/4] conf: Use correct type for balloon stats period
We're parsing memballoon status period as unsigned int, but when we're trying to set it, both we and qemu use signed int. That means large values will get wrapped around to negative one resulting in error. Basically the same problem as commit e3a7b874 was dealing with when updating live domain. QEMU changed the accepted value to int64 in commit 1f9296b5, but even values as INT_MAX don't make sense since the value passed means seconds. Hence adding capability flag for this change isn't worth it. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140958 Signed-off-by: Luyao Huang lhu...@redhat.com Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/formatdomain.html.in | 2 ++ src/conf/domain_conf.c| 9 +++-- src/conf/domain_conf.h| 2 +- src/qemu/qemu_process.c | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 40e2b29..7a11cc7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5630,6 +5630,8 @@ qemu-kvm -net nic,model=? /dev/null only be made to the active guest. If the QEMU driver is not at the right revision, the attempt to set the period will fail. + Large values might be ignored, but this only affects + non-sensical numbers (i.e. many years). span class='since'Since 1.1.1, requires QEMU 1.5/span /p /dd diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e010040..b3d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10432,6 +10432,7 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, char *model; virDomainMemballoonDefPtr def; xmlNodePtr save = ctxt-node; +unsigned int period = 0; if (VIR_ALLOC(def) 0) return NULL; @@ -10450,12 +10451,16 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, } ctxt-node = node; -if (virXPathUInt(string(./stats/@period), ctxt, def-period) -1) { +if (virXPathUInt(string(./stats/@period), ctxt, period) -1) { virReportError(VIR_ERR_XML_ERROR, %s, _(invalid statistics collection period)); goto error; } +def-period = period; +if (def-period 0) +def-period = 0; + if (def-model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) VIR_DEBUG(Ignoring device address for none model Memballoon); else if (virDomainDeviceInfoParseXML(node, NULL, def-info, flags) 0) @@ -18823,7 +18828,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf, virBufferAdjustIndent(childrenBuf, indent + 2); if (def-period) -virBufferAsprintf(childrenBuf, stats period='%u'/\n, def-period); +virBufferAsprintf(childrenBuf, stats period='%i'/\n, def-period); if (virDomainDeviceInfoNeedsFormat(def-info, flags) virDomainDeviceInfoFormat(childrenBuf, def-info, flags) 0) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ea463cb..ee0f5fd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1556,7 +1556,7 @@ enum { struct _virDomainMemballoonDef { int model; virDomainDeviceInfo info; -unsigned int period; /* seconds between collections */ +int period; /* seconds between collections */ }; struct _virDomainNVRAMDef { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d1f089d..0f357d5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4390,7 +4390,7 @@ int qemuProcessStart(virConnectPtr conn, virCommandPtr cmd = NULL; struct qemuProcessHookData hookData; unsigned long cur_balloon; -unsigned int period = 0; +int period = 0; size_t i; bool rawio_set = false; char *nodeset = NULL; -- 2.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/4] conf: Reorder elements inside memballoon
All the devices we have format their address as its last sub-element, so let's change memballoon to follow suit. Also adjust RNG to allow any order of them so 'virsh edit' doesn't shout at us. Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/schemas/domaincommon.rng | 28 ++-- src/conf/domain_conf.c | 30 ++ .../qemuxml2xmlout-balloon-device-period.xml | 30 ++ tests/qemuxml2xmltest.c| 1 + 4 files changed, 60 insertions(+), 29 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-period.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b1d883f..b9d430a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3438,19 +3438,21 @@ valuenone/value /choice /attribute - optional -ref name=alias/ - /optional - optional -ref name=address/ - /optional - optional -element name=stats - attribute name=period -ref name=positiveInteger/ - /attribute -/element - /optional + interleave +optional + ref name=alias/ +/optional +optional + ref name=address/ +/optional +optional + element name=stats +attribute name=period + ref name='positiveInteger'/ +/attribute + /element +/optional + /interleave /element /define define name=parallel diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ae8688e..e010040 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18810,7 +18810,8 @@ virDomainMemballoonDefFormat(virBufferPtr buf, unsigned int flags) { const char *model = virDomainMemballoonModelTypeToString(def-model); -bool noopts = true; +virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; +int indent = virBufferGetIndent(buf, false); if (!model) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -18819,27 +18820,24 @@ virDomainMemballoonDefFormat(virBufferPtr buf, } virBufferAsprintf(buf, memballoon model='%s', model); -virBufferAdjustIndent(buf, 2); +virBufferAdjustIndent(childrenBuf, indent + 2); -if (virDomainDeviceInfoNeedsFormat(def-info, flags)) { -virBufferAddLit(buf, \n); -if (virDomainDeviceInfoFormat(buf, def-info, flags) 0) -return -1; -noopts = false; -} +if (def-period) +virBufferAsprintf(childrenBuf, stats period='%u'/\n, def-period); -if (def-period) { -if (noopts) -virBufferAddLit(buf, \n); -virBufferAsprintf(buf, stats period='%u'/\n, def-period); -noopts = false; +if (virDomainDeviceInfoNeedsFormat(def-info, flags) +virDomainDeviceInfoFormat(childrenBuf, def-info, flags) 0) { +virBufferFreeAndReset(childrenBuf); +return -1; } -virBufferAdjustIndent(buf, -2); -if (noopts) +if (!virBufferUse(childrenBuf)) { virBufferAddLit(buf, /\n); -else +} else { +virBufferAddLit(buf, \n); +virBufferAddBuffer(buf, childrenBuf); virBufferAddLit(buf, /memballoon\n); +} return 0; } diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-period.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-period.xml new file mode 100644 index 000..79e465a --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-period.xml @@ -0,0 +1,30 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='pci' index='0' model='pci-root'/ +controller type='ide' index='0'/ +memballoon model='virtio' + stats period='10'/ + address type='pci' domain='0x' bus='0x00' slot='0x12' function='0x0'/ +/memballoon + /devices +/domain diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 8e12e84..9e4b3a2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -354,6 +354,7 @@ mymain(void) /* These tests generate different XML */ DO_TEST_DIFFERENT(balloon-device-auto); +DO_TEST_DIFFERENT(balloon-device-period);
[libvirt] [PATCH 3/3] qemu: Disallow concurrent block jobs on a single disk
While qemu may be prepared to do this libvirt is not. Forbid the block ops until we fix our code. --- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 23 +++ src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 28 +--- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ea463cb..6f2df46 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -664,6 +664,7 @@ struct _virDomainDiskDef { int tray_status; /* enum virDomainDiskTray */ int removable; /* enum virTristateSwitch */ +bool blockjob; virStorageSourcePtr mirror; int mirrorState; /* enum virDomainDiskMirrorState */ int mirrorJob; /* virDomainBlockJobType */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d8a2087..ff4307b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2756,6 +2756,29 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, return ret; } + +bool +qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk) +{ +if (disk-mirror) { +virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, + _(disk '%s' already in active block job), + disk-dst); + +return true; +} + +if (disk-blockjob) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _(disk '%s' already in active block job), + disk-dst); +return true; +} + +return false; +} + + int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a7ebb47..41e075b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -414,6 +414,8 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +bool qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk); + void qemuDomObjEndAPI(virDomainObjPtr *vm); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1aed55f..864ee50 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4537,6 +4537,7 @@ processBlockJobEvent(virQEMUDriverPtr driver, disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, true, true)); +disk-blockjob = false; break; case VIR_DOMAIN_BLOCK_JOB_READY: @@ -4552,6 +4553,7 @@ processBlockJobEvent(virQEMUDriverPtr driver, VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : VIR_DOMAIN_DISK_MIRROR_STATE_NONE; disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; save = true; +disk-blockjob = false; break; case VIR_DOMAIN_BLOCK_JOB_LAST: @@ -16133,6 +16135,7 @@ qemuDomainBlockPivot(virConnectPtr conn, disk-mirror = NULL; disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; +disk-blockjob = false; } if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0) ret = -1; @@ -16233,12 +16236,9 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto endjob; disk = vm-def-disks[idx]; -if (mode == BLOCK_JOB_PULL disk-mirror) { -virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, - _(disk '%s' already in active block job), - disk-dst); +if (mode == BLOCK_JOB_PULL qemuDomainDiskBlockJobIsActive(disk)) goto endjob; -} + if (mode == BLOCK_JOB_ABORT) { if ((flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) !(async disk-mirror)) { @@ -16322,6 +16322,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, if (mode == BLOCK_JOB_ABORT disk-mirror) disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; goto endjob; +} else if (mode == BLOCK_JOB_PULL) { +disk-blockjob = true; } waitjob: @@ -16573,12 +16575,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (!device) goto endjob; disk = vm-def-disks[idx]; -if (disk-mirror) { -virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, - _(disk '%s' already in active block job), - disk-dst); +if (qemuDomainDiskBlockJobIsActive(disk)) goto endjob; -} if (!(virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DRIVE_MIRROR) virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC))) { @@ -16699,6 +16697,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, disk-mirror = mirror; mirror = NULL; disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; +disk-blockjob = true; if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0)
[libvirt] [PATCH 0/3] qemu: fix broken block job handling
Block job handling violates our usage of domain jobs and changes disk source definition behind our back. Peter Krempa (3): qemu: process: Export qemuProcessFindDomainDiskByAlias qemu: event: Don't fiddle with disk backing trees without a job qemu: Disallow concurrent block jobs on a single disk src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 23 +++ src/qemu/qemu_domain.h | 4 ++ src/qemu/qemu_driver.c | 170 +++- src/qemu/qemu_process.c | 131 +++-- src/qemu/qemu_process.h | 3 + 6 files changed, 211 insertions(+), 121 deletions(-) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/9] libxl: add tablet/mouse input device support
Marek Marczykowski-Górecki wrote: From: Marek Marczykowski marma...@invisiblethingslab.com Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- src/libxl/libxl_conf.c | 44 1 file changed, 44 insertions(+) Changes in v2: - rebase on 1.2.12+ - multiple devices support Changes in v3: - reduce code duplication diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 2321660..50ef9d8 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -749,6 +749,50 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_defbool_set(b_info-u.hvm.vnc.enable, 0); libxl_defbool_set(b_info-u.hvm.sdl.enable, 0); +if (def-ninputs) { +for (i = 0; i def-ninputs; i++) { +if (def-inputs[i]-bus != VIR_DOMAIN_INPUT_BUS_USB) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +_(libxenlight supports only USB input)); +return -1; +} +} +#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST +if (VIR_ALLOC_N(b_info-u.hvm.usbdevice_list, def-ninputs+1) 0) +return -1; +#else +if (def-ninputs 1) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +_(libxenlight supports only one input device)); +return -1; +} +#endif +for (i = 0; i def-ninputs; i++) { +char **usbdevice; +#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST +usbdevice = b_info-u.hvm.usbdevice_list[i]; +#else +usbdevice = b_info-u.hvm.usbdevice; +#endif +switch (def-inputs[i]-type) { +case VIR_DOMAIN_INPUT_TYPE_MOUSE: +VIR_FREE(*usbdevice); +if (VIR_STRDUP(*usbdevice, mouse) 0) +return -1; +break; +case VIR_DOMAIN_INPUT_TYPE_TABLET: +VIR_FREE(*usbdevice); +if (VIR_STRDUP(*usbdevice, tablet) 0) +return -1; +break; +default: +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +_(Unknown input device type)); +return -1; +} +} +} + Looks good now; ACK. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: process: Export qemuProcessFindDomainDiskByAlias
--- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d1f089d..28c3c27 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -390,7 +390,7 @@ qemuProcessFindDomainDiskByPath(virDomainObjPtr vm, return NULL; } -static virDomainDiskDefPtr +virDomainDiskDefPtr qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, const char *alias) { diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 5c70803..3c04179 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -112,4 +112,7 @@ int qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, virDomainGraphicsDefPtr graphics, bool allocate); +virDomainDiskDefPtr qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, + const char *alias); + #endif /* __QEMU_PROCESS_H__ */ -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: event: Don't fiddle with disk backing trees without a job
Surprisingly we did not grab a VM job when a block job finished and we'd happily rewrite the backing chain data. This made it possible to crash libvirt when queueing two backing chains tightly and other badness. To fix it, add yet another handler to the helper thread that handles monitor events that require a job. --- src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 142 src/qemu/qemu_process.c | 129 --- 3 files changed, 168 insertions(+), 105 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index fe3e2b1..a7ebb47 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -196,6 +196,7 @@ typedef enum { QEMU_PROCESS_EVENT_DEVICE_DELETED, QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED, QEMU_PROCESS_EVENT_SERIAL_CHANGED, +QEMU_PROCESS_EVENT_BLOCK_JOB, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; @@ -204,6 +205,7 @@ struct qemuProcessEvent { virDomainObjPtr vm; qemuProcessEventType eventType; int action; +int status; void *data; }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3263ac..1aed55f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4448,6 +4448,141 @@ processSerialChangedEvent(virQEMUDriverPtr driver, } +static void +processBlockJobEvent(virQEMUDriverPtr driver, + virDomainObjPtr vm, + char *diskAlias, + int type, + int status) +{ +virObjectEventPtr event = NULL; +virObjectEventPtr event2 = NULL; +const char *path; +virDomainDiskDefPtr disk; +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +virDomainDiskDefPtr persistDisk = NULL; +bool save = false; + +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +VIR_DEBUG(Domain is not running); +goto endjob; +} + +disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); + +if (disk) { +/* Have to generate two variants of the event for old vs. new + * client callbacks */ +if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT +disk-mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) +type = disk-mirrorJob; +path = virDomainDiskGetSource(disk); +event = virDomainEventBlockJobNewFromObj(vm, path, type, status); +event2 = virDomainEventBlockJob2NewFromObj(vm, disk-dst, type, + status); + +/* If we completed a block pull or commit, then update the XML + * to match. */ +switch ((virConnectDomainEventBlockJobStatus) status) { +case VIR_DOMAIN_BLOCK_JOB_COMPLETED: +if (disk-mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) { +if (vm-newDef) { +int indx = virDomainDiskIndexByName(vm-newDef, disk-dst, +false); +virStorageSourcePtr copy = NULL; + +if (indx = 0) { +persistDisk = vm-newDef-disks[indx]; +copy = virStorageSourceCopy(disk-mirror, false); +if (virStorageSourceInitChainElement(copy, + persistDisk-src, + true) 0) { +VIR_WARN(Unable to update persistent definition + on vm %s after block job, + vm-def-name); +virStorageSourceFree(copy); +copy = NULL; +persistDisk = NULL; +} +} +if (copy) { +virStorageSourceFree(persistDisk-src); +persistDisk-src = copy; +} +} + +/* XXX We want to revoke security labels and disk + * lease, as well as audit that revocation, before + * dropping the original source. But it gets tricky + * if both source and mirror share common backing + * files (we want to only revoke the non-shared + * portion of the chain); so for now, we leak the + * access to the original. */ +virStorageSourceFree(disk-src); +disk-src = disk-mirror; +} else { +virStorageSourceFree(disk-mirror); +} + +/* Recompute the cached backing chain to match our + * updates. Better would be storing the chain ourselves + * rather than reprobing, but we haven't quite completed + * that conversion
Re: [libvirt] [PATCH 3/9] tests: xenconfig: test for multiple USB devices and other HVM options
Marek Marczykowski-Górecki wrote: Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- tests/xlconfigdata/test-fullvirt-multiusb.cfg | 29 tests/xlconfigdata/test-fullvirt-multiusb.xml | 48 +++ tests/xlconfigtest.c | 1 + 3 files changed, 78 insertions(+) create mode 100755 tests/xlconfigdata/test-fullvirt-multiusb.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-multiusb.xml Nice, thanks! I've tested this with the various combinations of --with{out}-xen --with{out}-libxl configure options. ACK. Regards, Jim diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.cfg b/tests/xlconfigdata/test-fullvirt-multiusb.cfg new file mode 100755 index 000..ba4bf52 --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-multiusb.cfg @@ -0,0 +1,29 @@ +name = XenGuest2 +uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809 +maxmem = 579 +memory = 394 +vcpus = 1 +builder = hvm +kernel = /usr/lib/xen/boot/hvmloader +boot = d +pae = 1 +acpi = 1 +apic = 1 +hap = 0 +viridian = 0 +localtime = 0 +on_poweroff = destroy +on_reboot = restart +on_crash = restart +device_model = /usr/lib/xen/bin/qemu-dm +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = 127.0.0.1 +vncpasswd = 123poi +vif = [ mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu ] +parallel = none +serial = none +disk = [ /dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy, /root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom ] +usb = 1 +usbdevice = [ mouse, tablet ] diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.xml b/tests/xlconfigdata/test-fullvirt-multiusb.xml new file mode 100644 index 000..642c242 --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-multiusb.xml @@ -0,0 +1,48 @@ +domain type='xen' + nameXenGuest2/name + uuidc7a5fdb2-cdaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'592896/memory + currentMemory unit='KiB'403456/currentMemory + vcpu placement='static'1/vcpu + os +type arch='x86_64' machine='xenfv'hvm/type +loader type='rom'/usr/lib/xen/boot/hvmloader/loader +boot dev='cdrom'/ + /os + features +acpi/ +apic/ +pae/ + /features + clock offset='utc' adjustment='reset'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashrestart/on_crash + devices +emulator/usr/lib/xen/bin/qemu-dm/emulator +disk type='block' device='disk' + driver name='phy' type='raw'/ + source dev='/dev/HostVG/XenGuest2'/ + target dev='hda' bus='ide'/ +/disk +disk type='file' device='cdrom' + driver name='qemu' type='raw'/ + source file='/root/boot.iso'/ + target dev='hdc' bus='ide'/ + readonly/ +/disk +interface type='bridge' + mac address='00:16:3e:66:92:9c'/ + source bridge='xenbr1'/ + script path='vif-bridge'/ + model type='e1000'/ +/interface +input type='mouse' bus='usb'/ +input type='tablet' bus='usb'/ +input type='mouse' bus='ps2'/ +input type='keyboard' bus='ps2'/ +graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi' + listen type='address' address='127.0.0.1'/ +/graphics + /devices +/domain diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 8c4c82c..6d4aa6d 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -215,6 +215,7 @@ mymain(void) DO_TEST(new-disk, 3); DO_TEST(spice, 3); +DO_TEST(fullvirt-multiusb, 3); virObjectUnref(caps); virObjectUnref(xmlopt); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list