Re: [libvirt] [PATCH 0/2] target-i386: Haswell-noTSX and Broadwell-noTSX CPU models

2015-03-13 Thread Andreas Färber
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

2015-03-13 Thread Eduardo Habkost
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

2015-03-13 Thread 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

 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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread Jim Fehlig
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

2015-03-13 Thread Jim Fehlig
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

2015-03-13 Thread Jim Fehlig
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

2015-03-13 Thread 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, 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

2015-03-13 Thread Jim Fehlig
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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread Jim Fehlig
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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread Eduardo Habkost
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

2015-03-13 Thread Jim Fehlig
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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan
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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread Marek Marczykowski-Górecki
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

2015-03-13 Thread Jim Fehlig
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

2015-03-13 Thread John Ferlan

 +
 +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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread hongming

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

2015-03-13 Thread zhang bo
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

2015-03-13 Thread Peter Krempa
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

2015-03-13 Thread Paolo Bonzini


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

2015-03-13 Thread Luyao Huang
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?

2015-03-13 Thread Daniel P. Berrange
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

2015-03-13 Thread Peter Krempa
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?

2015-03-13 Thread zhang bo
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

2015-03-13 Thread Peter Krempa
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?

2015-03-13 Thread zhang bo
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?

2015-03-13 Thread zhang bo
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

2015-03-13 Thread hongming

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

2015-03-13 Thread 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.

 
 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

2015-03-13 Thread Peter Krempa
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

2015-03-13 Thread Peter Krempa
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

2015-03-13 Thread zhanghailiang
'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

2015-03-13 Thread Maxim Nestratov

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

2015-03-13 Thread Peter Krempa
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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread Martin Kletzander

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

2015-03-13 Thread John Ferlan


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

2015-03-13 Thread Dr. David Alan Gilbert
* 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

2015-03-13 Thread Peter Krempa
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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread Daniel P. Berrange
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Michal Privoznik
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Taowei Luo
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

2015-03-13 Thread Peter Krempa
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

2015-03-13 Thread Michal Privoznik
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Martin Kletzander
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

2015-03-13 Thread Martin Kletzander
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

2015-03-13 Thread Martin Kletzander
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Maxim Nestratov
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

2015-03-13 Thread Eric Blake
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

2015-03-13 Thread Martin Kletzander
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

2015-03-13 Thread Martin Kletzander
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

2015-03-13 Thread Peter Krempa
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

2015-03-13 Thread Peter Krempa
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

2015-03-13 Thread Jim Fehlig
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

2015-03-13 Thread Peter Krempa
---
 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

2015-03-13 Thread Peter Krempa
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

2015-03-13 Thread Jim Fehlig
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