[libvirt] [PATCH v3] qemu: Use heads parameter for QXL driver

2015-07-06 Thread Frediano Ziglio
Allows to specify maximum number of head to QXL driver.

The patch to support the max_outputs in Qemu is still not merged but
I got agreement on the name of the argument.

Actually can be a compatiblity problem as heads in the XML configuration
was set by default to '1'.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 src/qemu/qemu_command.c  | 5 +
 3 files changed, 8 insertions(+)

Changes from v2:
- removed capability tests (Martin Kletzander)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 27686c3..68060cd 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -287,6 +287,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   aarch64-off,
 
   vhost-user-multiqueue, /* 190 */
+  qxl-vga.max_outputs,
 );
 
 
@@ -1649,6 +1650,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsQxl[] = {
 
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = {
 { vgamem_mb, QEMU_CAPS_QXL_VGA_VGAMEM },
+{ max_outputs, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS },
 };
 
 struct virQEMUCapsObjectTypeProps {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 30aa504..02f9e81 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -230,6 +230,7 @@ typedef enum {
 QEMU_CAPS_DEVICE_PCI_SERIAL  = 188, /* -device pci-serial */
 QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */
 QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */
+QEMU_CAPS_QXL_VGA_MAX_OUTPUTS = 191, /* qxl-vga.max_outputs */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 25a7bc6..59666e7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5661,6 +5661,11 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def,
 /* QEMU accepts mebibytes for vgamem_mb. */
 virBufferAsprintf(buf, ,vgamem_mb=%u, video-vgamem / 1024);
 }
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) 
+video-heads  0) {
+virBufferAsprintf(buf, ,max_outputs=%u, video-heads);
+}
 } else if (video-vram 
 ((video-type == VIR_DOMAIN_VIDEO_TYPE_VGA 
   virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 2/2] Allow PCI virtio on ARM virt machine

2015-07-06 Thread Gerd Hoffmann
  Hi,

  I am working on tests, and i have discovered some issues with my current 
 implementation.
  Whether i concentrate my efforts on fixing them or just redo everything from 
 scratch, depends on
 what you say no. So what is the final verdict? Should we:
 a) Change the default to PCI, for better performance; or:
 b) Leave the default as it is, but make explicit address type='pci' working?

Clearly (b).  For the default it is important that it is backward
compatible (to older libvirt versions) and works with as many guests as
possible.

You might want to have a look at libosinfo which records (among other
information) which guest can handle which device type, exactly to allow
this kind of optimization.

cheers,
  Gerd


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 2/2] Allow PCI virtio on ARM virt machine

2015-07-06 Thread Pavel Fedin
 Hi!

 Clearly (b).  For the default it is important that it is backward
 compatible (to older libvirt versions) and works with as many guests as
 possible.

 One very last counter-argument...
 Actually, even if we change our default, we are still backwards-compatible, 
because older libvirt automatically adds address type='virtio-mmio'/ to the 
XML. So, all XMLs, created before the upgrade, will have it, so everything will 
keep working. The only thing affected by the change is domains created after 
the upgrade. You'll need to specify address type='virtio-mmio'/ explicitly in 
order to work with older OSes.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 03/10] qemu: Refactor creation of shared memory device commandline

2015-07-06 Thread lhuang


On 07/06/2015 01:38 PM, Martin Kletzander wrote:

On Mon, Jul 06, 2015 at 10:23:59AM +0800, lhuang wrote:


On 07/03/2015 08:56 PM, Martin Kletzander wrote:

On Wed, Jun 17, 2015 at 11:56:14AM +0800, Luyao Huang wrote:

Rename qemuBuildShmemDevCmd to qemuBuildShmemDevStr and change the
return type so that it can be reused in the device hotplug code later.

And split the chardev creation part in a new function
qemuBuildShmemBackendStr for reused in the device hotplug code later.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_command.c | 70
+++--
src/qemu/qemu_command.h |  7 +
2 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 636e040..0414f77 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8433,9 +8433,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
   return ret;
}

-static int
-qemuBuildShmemDevCmd(virCommandPtr cmd,
- virDomainDefPtr def,
+char *
+qemuBuildShmemDevStr(virDomainDefPtr def,
virDomainShmemDefPtr shmem,
virQEMUCapsPtr qemuCaps)
{
@@ -8489,14 +8488,38 @@ qemuBuildShmemDevCmd(virCommandPtr cmd,
   if (virBufferCheckError(buf)  0)
   goto error;

-virCommandAddArg(cmd, -device);
-virCommandAddArgBuffer(cmd, buf);
-
-return 0;
+return virBufferContentAndReset(buf);



You should be able to just unconditionally do
return virBufferContentAndReset() here since it returns NULL if
there's a buf-error.

ACK with that changed.



Right, i forgot that, thanks a lot for your review



Sorry, you cannot, there's a goto error; from some part of the code
where there is no buf-error set, so no change to this, you were
right.

No need to resend these first three, I'll push whatever is OK to go in
after I finish the review, and let you know what needs work.



Okay, got it, thanks a lot for your helps.

BR,
Luyao


Thanks,
Martin


Luyao


error:
   virBufferFreeAndReset(buf);
-return -1;
+return NULL;
+}
+
+char *
+qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem,
+ virQEMUCapsPtr qemuCaps)
+{
+char *devstr = NULL;
+virDomainChrSourceDef source = {
+.type = VIR_DOMAIN_CHR_TYPE_UNIX,
+.data.nix = {
+.path = shmem-server.path,
+.listen = false,
+}
+};
+
+if (!shmem-server.path 
+virAsprintf(source.data.nix.path,
+/var/lib/libvirt/shmem-%s-sock,
+shmem-name)  0)
+return NULL;
+
+devstr = qemuBuildChrChardevStr(source, shmem-info.alias,
qemuCaps);
+
+if (!shmem-server.path)
+VIR_FREE(source.data.nix.path);
+
+return devstr;
}

static int
@@ -8505,35 +8528,18 @@ qemuBuildShmemCommandLine(virCommandPtr cmd,
 virDomainShmemDefPtr shmem,
 virQEMUCapsPtr qemuCaps)
{
-if (qemuBuildShmemDevCmd(cmd, def, shmem, qemuCaps)  0)
+char *devstr = NULL;
+
+if (!(devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps)))
   return -1;
+virCommandAddArgList(cmd, -device, devstr, NULL);
+VIR_FREE(devstr);

   if (shmem-server.enabled) {
-char *devstr = NULL;
-virDomainChrSourceDef source = {
-.type = VIR_DOMAIN_CHR_TYPE_UNIX,
-.data.nix = {
-.path = shmem-server.path,
-.listen = false,
-}
-};
-
-if (!shmem-server.path 
-virAsprintf(source.data.nix.path,
-/var/lib/libvirt/shmem-%s-sock,
-shmem-name)  0)
+if (!(devstr = qemuBuildShmemBackendStr(shmem, qemuCaps)))
   return -1;

-devstr = qemuBuildChrChardevStr(source,
shmem-info.alias, qemuCaps);
-
-if (!shmem-server.path)
-VIR_FREE(source.data.nix.path);
-
-if (!devstr)
-return -1;
-
-virCommandAddArg(cmd, -chardev);
-virCommandAddArg(cmd, devstr);
+virCommandAddArgList(cmd, -chardev, devstr, NULL);
   VIR_FREE(devstr);
   }

diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 0fc59a8..73f24dc 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -194,6 +194,13 @@ int
qemuBuildRNGBackendProps(virDomainRNGDefPtr rng,
const char **type,
virJSONValuePtr *props);

+char *qemuBuildShmemDevStr(virDomainDefPtr def,
+   virDomainShmemDefPtr shmem,
+   virQEMUCapsPtr qemuCaps);
+char *qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem,
+   virQEMUCapsPtr qemuCaps);
+
+
int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);

/* Legacy, pre device support */
--
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list




--
libvir-list mailing list

[libvirt] [PATCH RESEND] Added support for portable-rpcgen from portablexdr library

2015-07-06 Thread Pavel Fedin
This patch allows to build libvirt natively under MinGW/MSYS using portablexdr 
library.
An updated version of portablexdr with fixed bugs is available as part of MSYS2 
project.

Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
 configure.ac   | 2 +-
 src/lxc/lxc_monitor_protocol.x | 2 +-
 src/rpc/genprotocol.pl | 5 -
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index aed0934..547a405 100644
--- a/configure.ac
+++ b/configure.ac
@@ -397,7 +397,7 @@ AM_CONDITIONAL([HAVE_LIBTASN1], [test 
x$ac_cv_header_libtasn1_h = xyes])
 AC_CHECK_LIB([intl],[gettext],[])
 
 dnl Do we have rpcgen?
-AC_PATH_PROG([RPCGEN], [rpcgen], [no])
+AC_PATH_PROGS([RPCGEN], [rpcgen portable-rpcgen], [no])
 AM_CONDITIONAL([HAVE_RPCGEN], [test x$ac_cv_path_RPCGEN != xno])
 dnl Is this GLIBC's buggy rpcgen?
 AM_CONDITIONAL([HAVE_GLIBC_RPCGEN],
diff --git a/src/lxc/lxc_monitor_protocol.x b/src/lxc/lxc_monitor_protocol.x
index 3b66af5..205d7c2 100644
--- a/src/lxc/lxc_monitor_protocol.x
+++ b/src/lxc/lxc_monitor_protocol.x
@@ -30,7 +30,7 @@ enum virLXCMonitorExitStatus {
 };
 
 struct virLXCMonitorExitEventMsg {
-enum virLXCMonitorExitStatus status;
+virLXCMonitorExitStatus status;
 };
 
 struct virLXCMonitorInitEventMsg {
diff --git a/src/rpc/genprotocol.pl b/src/rpc/genprotocol.pl
index 6e6d6d4..1ac2507 100755
--- a/src/rpc/genprotocol.pl
+++ b/src/rpc/genprotocol.pl
@@ -38,7 +38,10 @@ my $target = shift;
 
 unlink $target;
 
-open RPCGEN, -|, $rpcgen, $mode, $xdrdef
+if ($rpcgen =~ /portable-rpcgen/) {
+$rpcgen = $rpcgen -o -;
+}
+open RPCGEN, -|, $rpcgen $mode $xdrdef
 or die cannot run $rpcgen $mode $xdrdef: $!;
 open TARGET, $target
 or die cannot create $target: $!;
-- 
1.9.5.msysgit.0


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 2/2] Allow PCI virtio on ARM virt machine

2015-07-06 Thread Pavel Fedin
 Hello!

  I'm talking about the distro kernel as a guest. My understanding is that
  Fedora 21/22 AArch64 does _not_ work with virtio-pci, but it does work with
  virtio-mmio. But I've yet to confirm yet...
 
 arm64 has no generic pci host support yet (as of upstream kernel 4.1).
 arm has it for a while (and at least f22 works just fine with it).

 I am working on tests, and i have discovered some issues with my current 
implementation.
 Whether i concentrate my efforts on fixing them or just redo everything from 
scratch, depends on
what you say no. So what is the final verdict? Should we:
a) Change the default to PCI, for better performance; or:
b) Leave the default as it is, but make explicit address type='pci' working?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v3 1/2] Fix nodeinfo output on PPC64 KVM hosts

2015-07-06 Thread Shivaprasad bhat
Thanks a lot Andrea. The cleanups are really nice. I had a chance to
test the patch and it
seems to work consistently in all sucores_per_core modes.

Only two comments written inline .

Thanks and Regards,
Shiva

On Fri, Jul 3, 2015 at 5:57 PM, Andrea Bolognani abolo...@redhat.com wrote:
 From: Shivaprasad G Bhat sb...@linux.vnet.ibm.com

 The nodeinfo is reporting incorrect number of cpus and incorrect host
 topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
 the primary thread in a core to be online, and the secondaries offlined.
 While scheduling a guest in, the kvm scheduler wakes up the secondaries to
 run in guest context.

 The host scheduling of the guests happen at the core level(as only primary
 thread is online). The kvm scheduler exploits as many threads of the core
 as needed by guest. Further, starting POWER8, the processor allows splitting
 a physical core into multiple subcores with 2 or 4 threads each. Again, only
 the primary thread in a subcore is online in the host. The KVM-PPC
 scheduler allows guests to exploit all the offline threads in the subcore,
 by bringing them online when needed.
 (Kernel patches on split-core 
 http://www.spinics.net/lists/kvm-ppc/msg09121.html)

 Recently with dynamic micro-threading changes in ppc-kvm, makes sure
 to utilize all the offline cpus across guests, and across guests with
 different cpu topologies.
 (https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)

 Since the offline cpus are brought online in the guest context, it is safe
 to count them as online. Nodeinfo today discounts these offline cpus from
 cpu count/topology calclulation, and the nodeinfo output is not of any help
 and the host appears overcommited when it is actually not.

 The patch carefully counts those offline threads whose primary threads are
 online. The host topology displayed by the nodeinfo is also fixed when the
 host is in valid kvm state.

 Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
 Signed-off-by: Andrea Bolognani abolo...@redhat.com
 ---
  src/libvirt_private.syms |   1 +
  src/nodeinfo.c   | 140 
 ++-
  src/nodeinfo.h   |   1 +
  3 files changed, 129 insertions(+), 13 deletions(-)

 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 1566d11..64644a2 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1008,6 +1008,7 @@ nodeGetInfo;
  nodeGetMemory;
  nodeGetMemoryParameters;
  nodeGetMemoryStats;
 +nodeGetThreadsPerSubcore;

The nodeGetThreadsPerSubcore being PPC specific, is it good
to have it in nodeinfo.h ? That was the rational on which I had the
ioctl wrapper
written in virarch.c in v2.


  nodeSetMemoryParameters;


 diff --git a/src/nodeinfo.c b/src/nodeinfo.c
 index 2fafe2d..25a6471 100644
 --- a/src/nodeinfo.c
 +++ b/src/nodeinfo.c
 @@ -32,6 +32,12 @@
  #include sys/utsname.h
  #include sched.h
  #include conf/domain_conf.h
 +#include fcntl.h
 +#include sys/ioctl.h
 +
 +#if HAVE_LINUX_KVM_H
 +# include linux/kvm.h
 +#endif

  #if defined(__FreeBSD__) || defined(__APPLE__)
  # include sys/time.h
 @@ -428,28 +434,88 @@ virNodeParseNode(const char *node,
  unsigned int cpu;
  int online;
  int direrr;
 +int lastonline;
 +virBitmapPtr cpu_map = NULL;
 +int threads_per_subcore = 0;

  *threads = 0;
  *cores = 0;
  *sockets = 0;

 +/* PPC-KVM needs the secondary threads of a core to be offline on the
 + * host. The kvm scheduler brings the secondary threads online in the
 + * guest context. Moreover, P8 processor has split-core capability
 + * where, there can be 1,2 or 4 subcores per core. The primaries of the
 + * subcores alone will be online on the host for a subcore in the
 + * host. Even though the actual threads per core for P8 processor is 8,
 + * depending on the subcores_per_core = 1, 2 or 4, the threads per
 + * subcore will vary accordingly to 8, 4 and 2 repectively.
 + * So, On host threads_per_core what is arrived at from sysfs in the
 + * current logic is actually the subcores_per_core. Threads per subcore
 + * can only be obtained from the kvm device. For example, on P8 wih 1
 + * core having 8 threads, sub_cores_percore=4, the threads 0,2,4  6
 + * will be online. The sysfs reflects this and in the current logic
 + * variable 'threads' will be 4 which is nothing but subcores_per_core.
 + * If the user tampers the cpu online/offline states using chcpu or other
 + * means, then it is an unsupported configuration for kvm.
 + * The code below tries to keep in mind
 + *  - when the libvirtd is run inside a KVM guest or Phyp based guest.
 + *  - Or on the kvm host where user manually tampers the cpu states to
 + *offline/online randomly.
 + * On hosts other than POWER this will be 0, in which case a simpler
 + * thread-counting logic will be used  */
 +if ((threads_per_subcore 

[libvirt] [PATCH v3 3/3] Build correct command line for PCI NICs on ARM

2015-07-06 Thread Pavel Fedin
Legacy -net option works correctly only with embedded device models, which
do not require any bus specification. Therefore, we should use -device for
PCI hardware

Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
 src/qemu/qemu_command.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 25a7bc6..983fcdb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -457,7 +457,8 @@ qemuDomainSupportsNicdev(virDomainDefPtr def,
 /* non-virtio ARM nics require legacy -net nic */
 if (((def-os.arch == VIR_ARCH_ARMV7L) ||
 (def-os.arch == VIR_ARCH_AARCH64)) 
-net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
+net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO 
+net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
 return false;
 
 return true;
-- 
1.9.5.msysgit.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] vz: fix building capabilities

2015-07-06 Thread Maxim Nestratov

03.07.2015 20:26, Dmitry Guryanov пишет:

There should be at least one domain for each guest
in cababilities. And in current code we don't add
domain for this guest for example.

 if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,
  VIR_ARCH_X86_64,
  vz,
  NULL, 0, NULL)) == NULL)

Anyway, with two virt types it looks a litte messy, so let's
move adding guest and domain to a separate function.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
  src/vz/vz_driver.c | 92 ++
  1 file changed, 38 insertions(+), 54 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 47c5023..8c3c818 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -78,14 +78,45 @@ vzDriverUnlock(vzConnPtr driver)
  virMutexUnlock(driver-lock);
  }
  
+static int

+vzCapsAddGuestDomain(virCapsPtr caps,
+ virDomainOSType ostype,
+ virArch arch,
+ const char * emulator,
+ virDomainVirtType virt_type)
+{
+virCapsGuestPtr guest;
+
+if ((guest = virCapabilitiesAddGuest(caps, ostype, arch, emulator,
+ NULL, 0, NULL)) == NULL)
+return -1;
+
+
+if (virCapabilitiesAddGuestDomain(guest, virt_type,
+  NULL, NULL, 0, NULL) == NULL)
+return -1;
+
+return 0;
+}
+
  static virCapsPtr
  vzBuildCapabilities(void)
  {
  virCapsPtr caps = NULL;
  virCPUDefPtr cpu = NULL;
  virCPUDataPtr data = NULL;
-virCapsGuestPtr guest;
  virNodeInfo nodeinfo;
+virDomainOSType ostypes[] = {
+VIR_DOMAIN_OSTYPE_HVM,
+VIR_DOMAIN_OSTYPE_EXE
+};
+virArch archs[] = { VIR_ARCH_I686, VIR_ARCH_X86_64 };
+const char *const emulators[] = { parallels, vz };
+virDomainVirtType virt_types[] = {
+VIR_DOMAIN_VIRT_PARALLELS,
+VIR_DOMAIN_VIRT_VZ
+};
+size_t i, j, k;
  
  if ((caps = virCapabilitiesNew(virArchFromHost(),

 false, false)) == NULL)
@@ -94,59 +125,12 @@ vzBuildCapabilities(void)
  if (nodeCapsInitNUMA(caps)  0)
  goto error;
  
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,

- VIR_ARCH_X86_64,
- parallels,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,
- VIR_ARCH_I686,
- parallels,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-
-if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_PARALLELS,
-  NULL, NULL, 0, NULL) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_EXE,
- VIR_ARCH_X86_64,
- parallels,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_PARALLELS,
-  NULL, NULL, 0, NULL) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,
- VIR_ARCH_X86_64,
- vz,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,
- VIR_ARCH_I686,
- vz,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-
-if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_VZ,
-  NULL, NULL, 0, NULL) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_EXE,
- VIR_ARCH_X86_64,
- vz,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_VZ,
-  NULL, NULL, 0, NULL) == NULL)
-goto error;
+for (i = 0; i  2; i++)
+for (j = 0; j  2; j++)
+for (k = 0; k  2; k++)
+if (vzCapsAddGuestDomain(caps, ostypes[i], archs[j],
+ emulators[k], virt_types[k])  0)
+goto error;
  
  if (nodeGetInfo(nodeinfo))

  goto error;

ACK. Looks good.

--
libvir-list mailing list

[libvirt] [PATCH v3 0/3] Allow PCI virtio on ARM virt machine

2015-07-06 Thread Pavel Fedin
Virt machine in qemu since v2.3.0 has PCI generic host controller, and can use
PCI devices. This provides performance improvement as well as vhost-net with
irqfd support for virtio-net. However libvirt currently does not allow ARM virt
machine to have PCI devices. This patchset adds the necessary support.

This version is completely reworked and uses different approach.

Changes since v2:
- Correctly model PCI Express bus on the machine. It is now possible to
  explicitly specify address-type='pci' with attributes. This allows to
  attach not only virtio, but any other PCI device to the model.
- Default is not changed and still mmio, for backwards compatibility with
  existing installations. PCI bus has to be explicitly specified.
- Check for the capability in correct place, in v2 it actually did not work
Changes since v1:
- Added capability based on qemu version number
- Recognize also virt- prefix

Pavel Fedin (3):
  Introduce QEMU_CAPS_ARM_VIRT_PCI
  Add PCI-Express root to ARM virt machine
  Build correct command line for PCI NICs on ARM

 src/qemu/qemu_capabilities.c |  5 +
 src/qemu/qemu_capabilities.h |  1 +
 src/qemu/qemu_command.c  |  3 ++-
 src/qemu/qemu_domain.c   | 12 
 4 files changed, 16 insertions(+), 5 deletions(-)

-- 
1.9.5.msysgit.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3 2/3] Add PCI-Express root to ARM virt machine

2015-07-06 Thread Pavel Fedin
Add PCI Express root complex if the corresponding capability is present

Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
 src/qemu/qemu_domain.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f9bf32c..36f411d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -981,7 +981,7 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = {
 static int
 qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
-   void *opaque ATTRIBUTE_UNUSED)
+   void *opaque)
 {
 bool addDefaultUSB = true;
 bool addImplicitSATA = false;
@@ -1030,12 +1030,16 @@ qemuDomainDefPostParse(virDomainDefPtr def,
 break;
 
 case VIR_ARCH_ARMV7L:
-   addDefaultUSB = false;
-   addDefaultMemballoon = false;
-   break;
 case VIR_ARCH_AARCH64:
addDefaultUSB = false;
addDefaultMemballoon = false;
+   if (STREQ(def-os.machine, virt) ||
+   STRPREFIX(def-os.machine, virt-)) {
+   virQEMUDriverPtr driver = opaque;
+   virQEMUCapsPtr qemuCaps =
+   virQEMUCapsCacheLookup(driver-qemuCapsCache, def-emulator);
+   addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_ARM_VIRT_PCI);
+   }
break;
 
 case VIR_ARCH_PPC64:
-- 
1.9.5.msysgit.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3 1/3] Introduce QEMU_CAPS_ARM_VIRT_PCI

2015-07-06 Thread Pavel Fedin
This capability specifies that virt machine on ARM has PCI controller.
Enabled when qemu version is at least 2.3.0.

Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
 src/qemu/qemu_capabilities.c | 5 +
 src/qemu/qemu_capabilities.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 27686c3..0dc034f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -287,6 +287,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   aarch64-off,
 
   vhost-user-multiqueue, /* 190 */
+  arm-virt-pci,
 );
 
 
@@ -3352,6 +3353,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 qemuCaps-version = 2003000)
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_AARCH64_OFF);
 
+/* 'virt' machine has PCI controller from v2.3.0 onwards */
+if (qemuCaps-version = 2003000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_ARM_VIRT_PCI);
+
 /* vhost-user supports multi-queue from v2.4.0 onwards,
  * but there is no way to query for that capability */
 if (qemuCaps-version = 2004000)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 30aa504..f4180a8 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -230,6 +230,7 @@ typedef enum {
 QEMU_CAPS_DEVICE_PCI_SERIAL  = 188, /* -device pci-serial */
 QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */
 QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */
+QEMU_CAPS_ARM_VIRT_PCI   = 191, /* ARM 'virt' machine has PCI bus */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
-- 
1.9.5.msysgit.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] vz: fix building capabilities

2015-07-06 Thread Dmitry Guryanov

On 07/06/2015 03:45 PM, Maxim Nestratov wrote:

03.07.2015 20:26, Dmitry Guryanov пишет:

There should be at least one domain for each guest
in cababilities. And in current code we don't add
domain for this guest for example.

 if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,
  VIR_ARCH_X86_64,
  vz,
  NULL, 0, NULL)) == NULL)

Anyway, with two virt types it looks a litte messy, so let's
move adding guest and domain to a separate function.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
  src/vz/vz_driver.c | 92 
++

  1 file changed, 38 insertions(+), 54 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 47c5023..8c3c818 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -78,14 +78,45 @@ vzDriverUnlock(vzConnPtr driver)
  virMutexUnlock(driver-lock);
  }
  +static int
+vzCapsAddGuestDomain(virCapsPtr caps,
+ virDomainOSType ostype,
+ virArch arch,
+ const char * emulator,
+ virDomainVirtType virt_type)
+{
+virCapsGuestPtr guest;
+
+if ((guest = virCapabilitiesAddGuest(caps, ostype, arch, emulator,
+ NULL, 0, NULL)) == NULL)
+return -1;
+
+
+if (virCapabilitiesAddGuestDomain(guest, virt_type,
+  NULL, NULL, 0, NULL) == NULL)
+return -1;
+
+return 0;
+}
+
  static virCapsPtr
  vzBuildCapabilities(void)
  {
  virCapsPtr caps = NULL;
  virCPUDefPtr cpu = NULL;
  virCPUDataPtr data = NULL;
-virCapsGuestPtr guest;
  virNodeInfo nodeinfo;
+virDomainOSType ostypes[] = {
+VIR_DOMAIN_OSTYPE_HVM,
+VIR_DOMAIN_OSTYPE_EXE
+};
+virArch archs[] = { VIR_ARCH_I686, VIR_ARCH_X86_64 };
+const char *const emulators[] = { parallels, vz };
+virDomainVirtType virt_types[] = {
+VIR_DOMAIN_VIRT_PARALLELS,
+VIR_DOMAIN_VIRT_VZ
+};
+size_t i, j, k;
if ((caps = virCapabilitiesNew(virArchFromHost(),
 false, false)) == NULL)
@@ -94,59 +125,12 @@ vzBuildCapabilities(void)
  if (nodeCapsInitNUMA(caps)  0)
  goto error;
  -if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,
- VIR_ARCH_X86_64,
- parallels,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,
- VIR_ARCH_I686,
- parallels,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-
-if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_PARALLELS,
-  NULL, NULL, 0, NULL) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_EXE,
- VIR_ARCH_X86_64,
- parallels,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_PARALLELS,
-  NULL, NULL, 0, NULL) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,
- VIR_ARCH_X86_64,
- vz,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM,
- VIR_ARCH_I686,
- vz,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-
-if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_VZ,
-  NULL, NULL, 0, NULL) == NULL)
-goto error;
-
-if ((guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_EXE,
- VIR_ARCH_X86_64,
- vz,
- NULL, 0, NULL)) == NULL)
-goto error;
-
-if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_VZ,
-  NULL, NULL, 0, NULL) == NULL)
-goto error;
+for (i = 0; i  2; i++)
+for (j = 0; j  2; j++)
+for (k = 0; k  2; k++)
+if (vzCapsAddGuestDomain(caps, ostypes[i], archs[j],
+ emulators[k], 
virt_types[k])  0)

+goto error;
if (nodeGetInfo(nodeinfo))
  goto error;

ACK. Looks good.



Re: [libvirt] [libvirt-glib PATCHv4 7/7] gobject: Add wrapper for virNetworkGetDHCPLeases

2015-07-06 Thread Christophe Fergeau
On Wed, Jul 01, 2015 at 09:40:51PM +0100, Zeeshan Ali (Khattak) wrote:
 ---
  libvirt-gobject/libvirt-gobject-network.c | 54 
 +++
  libvirt-gobject/libvirt-gobject-network.h |  4 +++
  libvirt-gobject/libvirt-gobject.sym   |  2 ++
  3 files changed, 60 insertions(+)
 
 diff --git a/libvirt-gobject/libvirt-gobject-network.c 
 b/libvirt-gobject/libvirt-gobject-network.c
 index b1b38a0..b29be36 100644
 --- a/libvirt-gobject/libvirt-gobject-network.c
 +++ b/libvirt-gobject/libvirt-gobject-network.c
 @@ -29,6 +29,7 @@
  #include libvirt-glib/libvirt-glib.h
  #include libvirt-gobject/libvirt-gobject.h
  #include libvirt-gobject-compat.h
 +#include libvirt-gobject/libvirt-gobject-network-dhcp-lease-private.h
  
  #define GVIR_NETWORK_GET_PRIVATE(obj) \
  (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_NETWORK, 
 GVirNetworkPrivate))
 @@ -224,3 +225,56 @@ GVirConfigNetwork *gvir_network_get_config(GVirNetwork 
 *network,
  free(xml);
  return conf;
  }
 +
 +/**
 + * gvir_network_get_dhcp_leases:
 + * @network: the network
 + * @mac: (allow-none): The optional ASCII formatted MAC address of an 
 interface
 + * @flags: placeholder for flags, pass 0

must be 0 rather than pass 0 ?

 + *
 + * @err: Place-holder for possible errors
 + *
 + * This function fetches leases info of guests in the specified network. If 
 the
 + * optional parameter @mac is specified, the returned list will contain only
 + * lease info about a specific guest interface with @mac. There can be 
 multiple
 + * leases for a single @mac because this API supports DHCPv6 too.
 + *
 + * Returns:  (element-type LibvirtGObject.NetworkDHCPLease) (transfer full): 
 the
 + * list of network leases. Each object in the returned list should be 
 unreffed
 + * with g_object_unref() and the list itself using g_list_free, when no 
 longer
 + * needed.
 + */
 +GList *gvir_network_get_dhcp_leases(GVirNetwork *network,
 +const char* mac,
 +guint flags,
 +GError **err)
 +{
 +virNetworkDHCPLeasePtr *leases;
 +GList *ret = NULL;
 +int num_leases, i;
 +
 +g_return_val_if_fail(GVIR_IS_NETWORK(network), NULL);
 +g_return_val_if_fail(err == NULL || *err == NULL, NULL);

I'd add a g_return_val_if_fail(flags != 0, NULL);

ACK.

Christophe


pgplu1FcP1T3i.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-glib PATCHv4 1/7] gobject: Simplify gvir_connection_list*() implementations

2015-07-06 Thread Christophe Fergeau
On Wed, Jul 01, 2015 at 09:40:45PM +0100, Zeeshan Ali (Khattak) wrote:
 Make use of virConnectListAll* functions to avoid making 4 calls and
 hence avoid race conditions and complicated code.

Fwiw, I would have split this in 2, one for domains, and one for pools,
but fine this way too, ACK.

Christophe


pgp_BFSEmmCzR.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-glib PATCHv4 2/7] gobject: Plug 2 virConnect leaks

2015-07-06 Thread Christophe Fergeau
Please be a bit more verbose in the commit log as to what the leak is.
(a virConnect reference is leaked in error cases, the unref is moved
after the label we jump to on errors to avoid the leak)

Looks good otherwise,

Christophe

On Wed, Jul 01, 2015 at 09:40:46PM +0100, Zeeshan Ali (Khattak) wrote:
 ---
  libvirt-gobject/libvirt-gobject-connection.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
 b/libvirt-gobject/libvirt-gobject-connection.c
 index 1576906..dddbd3a 100644
 --- a/libvirt-gobject/libvirt-gobject-connection.c
 +++ b/libvirt-gobject/libvirt-gobject-connection.c
 @@ -748,7 +748,6 @@ gboolean gvir_connection_fetch_domains(GVirConnection 
 *conn,
  if (priv-domains)
  g_hash_table_unref(priv-domains);
  priv-domains = doms;
 -virConnectClose(vconn);
  g_mutex_unlock(priv-lock);
  
  ret = TRUE;
 @@ -759,6 +758,8 @@ cleanup:
  virDomainFree(domains[i]);
  free(domains);
  }
 +if (vconn != NULL)
 +virConnectClose(vconn);
  return ret;
  }
  
 @@ -835,7 +836,6 @@ gboolean 
 gvir_connection_fetch_storage_pools(GVirConnection *conn,
  if (priv-pools)
  g_hash_table_unref(priv-pools);
  priv-pools = pools;
 -virConnectClose(vconn);
  g_mutex_unlock(priv-lock);
  
  ret = TRUE;
 @@ -846,6 +846,8 @@ cleanup:
  virStoragePoolFree(vpools[i]);
  free(vpools);
  }
 +if (vconn != NULL)
 +virConnectClose(vconn);
  return ret;
  }
  
 -- 
 2.4.3
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


pgpGhQ__d5F8V.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 00/10] Restore code to allow unpriv_sgio for hostdev SCSI generic

2015-07-06 Thread John Ferlan
v1 here:
http://www.redhat.com/archives/libvir-list/2015-June/msg00814.html

Changes since v1:
- Add doc patch 1 to indicate that this feature may only be supported by
  certain kernels
- Adjust former patch 1 to add call to qemuIsSharedHostdev from
  qemuSetUnprivSGIO
- Insert patches 7  8 which essentially refactor qemuSetUnprivSGIO a bit.
  There should be no functional difference
- Patch 9 is now a much slimmer former patch 6

The end result is that 'generically speaking' if any kernel supports
setting the unprivileged SGIO feature, then these patches provide
the capability to do so.

Although as pointed out in the review of v1 only one specific downstream
kernel supports the feature, that doesn't mean other distros couldn't add
support in the same manner. So rather than just remove all traces from
libvirt completely, it seems it would be reasonable to keep the checks
in place and if a kernel then decides to add support this code exists
to assist.

John Ferlan (10):
  docs: Clarify unprivileged sgio feature for host devices
  qemu: Introduce qemuIsSharedHostdev
  qemu: Introduce qemuGetHostdevPath
  qemu: Refactor qemuCheckSharedDisk to create virCheckUnprivSGIO
  qemu: Refactor qemuAddSharedHostdev and qemuRemoveSharedHostdev
  qemu: Extract qemuGetHostdevPath from qemuGetSharedHostdevKey
  qemu: Refactor qemuSetUnprivSGIO return values
  qemu: Fix integer/boolean logic in qemuSetUnprivSGIO
  qemu: Add ability to set sgio values for hostdev
  qemu: Add check for unpriv sgio for SCSI generic host device

 docs/formatdomain.html.in |   7 +-
 src/qemu/qemu_conf.c  | 226 ++
 2 files changed, 154 insertions(+), 79 deletions(-)

-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 01/10] docs: Clarify unprivileged sgio feature for host devices

2015-07-06 Thread John Ferlan
Not all kernels support SG_IO for host devices, so let's indicate so

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatdomain.html.in | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ea2fff8..0fc5d85 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3232,11 +3232,12 @@
   /dd
   dtscsi/dt
   ddFor SCSI devices, user is responsible to make sure the device
-is not used by host. The optional codesgio/code
+is not used by host. If supported by the kernel,
+the optional codesgio/code
 (span class=sincesince 1.0.6/span) attribute indicates
 whether the kernel will filter unprivileged SG_IO commands for
-the disk, valid settings are filtered or unfiltered.
-The default is filtered. The optional coderawio/code
+the disk. Valid settings are filtered or unfiltered where
+the default is filtered. The optional coderawio/code
 (span class=sincesince 1.2.9/span) attribute indicates
 whether the lun needs the rawio capability. Valid settings are
 yes or no. See the rawio description within the
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 09/10] qemu: Add ability to set sgio values for hostdev

2015-07-06 Thread John Ferlan
Add necessary checks in order to allow setting sgio values for a scsi
host device

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_conf.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 589a6cf..7df971b 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1446,6 +1446,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 virDomainDiskDefPtr disk = NULL;
 virDomainHostdevDefPtr hostdev = NULL;
 char *sysfs_path = NULL;
+char *hostdev_path = NULL;
 const char *path = NULL;
 bool val = false;
 int ret = -1;
@@ -1467,13 +1468,10 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 if (!qemuIsSharedHostdev(hostdev))
 return 0;
 
-if (hostdev-source.subsys.u.scsi.sgio)
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _('sgio' is not supported for SCSI 
- generic device yet ));
+if (!(hostdev_path = qemuGetHostdevPath(hostdev)))
 goto cleanup;
 
-return 0;
+path = hostdev_path;
 } else {
 return 0;
 }
@@ -1482,7 +1480,11 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 goto cleanup;
 
 /* By default, filter the SG_IO commands, i.e. set unpriv_sgio to 0.  */
-val = (disk-sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED);
+if (dev-type == VIR_DOMAIN_DEVICE_DISK)
+val = (disk-sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED);
+else
+val = (hostdev-source.subsys.u.scsi.sgio ==
+   VIR_DOMAIN_DEVICE_SGIO_UNFILTERED);
 
 /* Do not do anything if unpriv_sgio is not supported by the kernel and the
  * whitelist is enabled.  But if requesting unfiltered access, always call
@@ -1499,6 +1501,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 ret = 0;
 
  cleanup:
+VIR_FREE(hostdev_path);
 VIR_FREE(sysfs_path);
 return ret;
 }
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 07/10] qemu: Refactor qemuSetUnprivSGIO return values

2015-07-06 Thread John Ferlan
Set to ret = -1 and prove otherwise, like usual

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_conf.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 80b8926..5ebf2cc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1448,7 +1448,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 char *sysfs_path = NULL;
 const char *path = NULL;
 int val = -1;
-int ret = 0;
+int ret = -1;
 
 /* sgio is only valid for block disk; cdrom
  * and floopy disk can have empty source.
@@ -1467,24 +1467,19 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 if (!qemuIsSharedHostdev(hostdev))
 return 0;
 
-if (hostdev-source.subsys.u.scsi.sgio) {
+if (hostdev-source.subsys.u.scsi.sgio)
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_('sgio' is not supported for SCSI 
  generic device yet ));
-ret = -1;
 goto cleanup;
-}
 
 return 0;
 } else {
 return 0;
 }
 
-sysfs_path = virGetUnprivSGIOSysfsPath(path, NULL);
-if (sysfs_path == NULL) {
-ret = -1;
+if (!(sysfs_path = virGetUnprivSGIOSysfsPath(path, NULL)))
 goto cleanup;
-}
 
 /* By default, filter the SG_IO commands, i.e. set unpriv_sgio to 0.  */
 val = (disk-sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED);
@@ -1495,7 +1490,9 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
  */
 if ((virFileExists(sysfs_path) || val == 1) 
 virSetDeviceUnprivSGIO(path, NULL, val)  0)
-ret = -1;
+goto cleanup;
+
+ret = 0;
 
  cleanup:
 VIR_FREE(sysfs_path);
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 08/10] qemu: Fix integer/boolean logic in qemuSetUnprivSGIO

2015-07-06 Thread John Ferlan
Setting of 'val' is a boolean expression, so handle it that way and
adjust the check/return logic to be clearer

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_conf.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5ebf2cc..589a6cf 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1447,7 +1447,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 virDomainHostdevDefPtr hostdev = NULL;
 char *sysfs_path = NULL;
 const char *path = NULL;
-int val = -1;
+bool val = false;
 int ret = -1;
 
 /* sgio is only valid for block disk; cdrom
@@ -1488,8 +1488,12 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
  * whitelist is enabled.  But if requesting unfiltered access, always call
  * virSetDeviceUnprivSGIO, to report an error for unsupported unpriv_sgio.
  */
-if ((virFileExists(sysfs_path) || val == 1) 
-virSetDeviceUnprivSGIO(path, NULL, val)  0)
+if (!val || !virFileExists(sysfs_path)) {
+ret = 0;
+goto cleanup;
+}
+
+if (virSetDeviceUnprivSGIO(path, NULL, 1)  0)
 goto cleanup;
 
 ret = 0;
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 03/10] qemu: Introduce qemuGetHostdevPath

2015-07-06 Thread John Ferlan
Introduce a convenience function to handle formulating the hostdev path

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_conf.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 48fb74a..f82244f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1213,15 +1213,13 @@ qemuIsSharedHostdev(virDomainHostdevDefPtr hostdev)
 return false;
 }
 
-
 static char *
-qemuGetSharedHostdevKey(virDomainHostdevDefPtr hostdev)
+qemuGetHostdevPath(virDomainHostdevDefPtr hostdev)
 {
 virDomainHostdevSubsysSCSIPtr scsisrc = hostdev-source.subsys.u.scsi;
 virDomainHostdevSubsysSCSIHostPtr scsihostsrc = scsisrc-u.host;
 char *dev_name = NULL;
 char *dev_path = NULL;
-char *key = NULL;
 
 if (!(dev_name = virSCSIDeviceGetDevName(NULL,
  scsihostsrc-adapter,
@@ -1230,14 +1228,27 @@ qemuGetSharedHostdevKey(virDomainHostdevDefPtr hostdev)
  scsihostsrc-unit)))
 goto cleanup;
 
-if (virAsprintf(dev_path, /dev/%s, dev_name)  0)
+ignore_value(virAsprintf(dev_path, /dev/%s, dev_name));
+
+ cleanup:
+VIR_FREE(dev_name);
+return dev_path;
+}
+
+
+static char *
+qemuGetSharedHostdevKey(virDomainHostdevDefPtr hostdev)
+{
+char *key = NULL;
+char *dev_path = NULL;
+
+if (!(dev_path = qemuGetHostdevPath(hostdev)))
 goto cleanup;
 
 if (!(key = qemuGetSharedDeviceKey(dev_path)))
 goto cleanup;
 
  cleanup:
-VIR_FREE(dev_name);
 VIR_FREE(dev_path);
 
 return key;
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 06/10] qemu: Extract qemuGetHostdevPath from qemuGetSharedHostdevKey

2015-07-06 Thread John Ferlan
The device path will be necessary for any future patch to allow sgio
settings on a hostdev

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_conf.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 8afbddc..80b8926 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1263,20 +1263,14 @@ qemuGetHostdevPath(virDomainHostdevDefPtr hostdev)
 
 
 static char *
-qemuGetSharedHostdevKey(virDomainHostdevDefPtr hostdev)
+qemuGetSharedHostdevKey(const char *dev_path)
 {
 char *key = NULL;
-char *dev_path = NULL;
-
-if (!(dev_path = qemuGetHostdevPath(hostdev)))
-goto cleanup;
 
 if (!(key = qemuGetSharedDeviceKey(dev_path)))
 goto cleanup;
 
  cleanup:
-VIR_FREE(dev_path);
-
 return key;
 }
 
@@ -1285,6 +1279,7 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
  virDomainHostdevDefPtr hostdev,
  const char *name)
 {
+char *dev_path = NULL;
 char *key = NULL;
 int ret = -1;
 
@@ -1293,7 +1288,10 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
 
 qemuDriverLock(driver);
 
-if (!(key = qemuGetSharedHostdevKey(hostdev)))
+if (!(dev_path = qemuGetHostdevPath(hostdev)))
+goto cleanup;
+
+if (!(key = qemuGetSharedHostdevKey(dev_path)))
 goto cleanup;
 
 if (qemuSharedDeviceEntryInsert(driver, key, name)  0)
@@ -1303,6 +1301,7 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
 
  cleanup:
 qemuDriverUnlock(driver);
+VIR_FREE(dev_path);
 VIR_FREE(key);
 return ret;
 }
@@ -1391,6 +1390,7 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver,
 virDomainHostdevDefPtr hostdev,
 const char *name)
 {
+char *dev_path = NULL;
 char *key = NULL;
 int ret = -1;
 
@@ -1399,7 +1399,10 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver,
 
 qemuDriverLock(driver);
 
-if (!(key = qemuGetSharedHostdevKey(hostdev)))
+if (!(dev_path = qemuGetHostdevPath(hostdev)))
+goto cleanup;
+
+if (!(key = qemuGetSharedHostdevKey(dev_path)))
 goto cleanup;
 
 if (qemuSharedDeviceEntryRemove(driver, key, name)  0)
@@ -1408,6 +1411,7 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver,
 ret = 0;
  cleanup:
 qemuDriverUnlock(driver);
+VIR_FREE(dev_path);
 VIR_FREE(key);
 return ret;
 }
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 04/10] qemu: Refactor qemuCheckSharedDisk to create virCheckUnprivSGIO

2015-07-06 Thread John Ferlan
Split out the SGIO check for sharing with hostdev in future patches

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_conf.c | 88 ++--
 1 file changed, 57 insertions(+), 31 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index f82244f..eb0b34f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1018,26 +1018,21 @@ qemuGetSharedDeviceKey(const char *device_path)
 return key;
 }
 
-/* Check if a shared device's setting conflicts with the conf
- * used by other domain(s). Currently only checks the sgio
- * setting. Note that this should only be called for disk with
- * block source if the device type is disk.
- *
- * Returns 0 if no conflicts, otherwise returns -1.
+/*
+ * Make necessary checks for the need to check and for the current setting
+ * of the 'unpriv_sgio' value for the device_path passed.
  */
 static int
-qemuCheckSharedDisk(virHashTablePtr sharedDevices,
-virDomainDiskDefPtr disk)
+virCheckUnprivSGIO(virHashTablePtr sharedDevices,
+   const char *device_path,
+   int sgio)
 {
 char *sysfs_path = NULL;
 char *key = NULL;
 int val;
 int ret = -1;
 
-if (disk-device != VIR_DOMAIN_DISK_DEVICE_LUN)
-return 0;
-
-if (!(sysfs_path = virGetUnprivSGIOSysfsPath(disk-src-path, NULL)))
+if (!(sysfs_path = virGetUnprivSGIOSysfsPath(device_path, NULL)))
 goto cleanup;
 
 /* It can't be conflict if unpriv_sgio is not supported by kernel. */
@@ -1046,7 +1041,7 @@ qemuCheckSharedDisk(virHashTablePtr sharedDevices,
 goto cleanup;
 }
 
-if (!(key = qemuGetSharedDeviceKey(disk-src-path)))
+if (!(key = qemuGetSharedDeviceKey(device_path)))
 goto cleanup;
 
 /* It can't be conflict if no other domain is sharing it. */
@@ -1055,29 +1050,19 @@ qemuCheckSharedDisk(virHashTablePtr sharedDevices,
 goto cleanup;
 }
 
-if (virGetDeviceUnprivSGIO(disk-src-path, NULL, val)  0)
+if (virGetDeviceUnprivSGIO(device_path, NULL, val)  0)
 goto cleanup;
 
+/* Error message on failure needs to be handled in caller
+ * since there is more specific knowledge of device
+ */
+virResetLastError();
 if (!((val == 0 
-   (disk-sgio == VIR_DOMAIN_DEVICE_SGIO_FILTERED ||
-disk-sgio == VIR_DOMAIN_DEVICE_SGIO_DEFAULT)) ||
+   (sgio == VIR_DOMAIN_DEVICE_SGIO_FILTERED ||
+sgio == VIR_DOMAIN_DEVICE_SGIO_DEFAULT)) ||
   (val == 1 
-   disk-sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED))) {
-
-if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_VOLUME) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _(sgio of shared disk 'pool=%s' 'volume=%s' 
conflicts 
- with other active domains),
-   disk-src-srcpool-pool,
-   disk-src-srcpool-volume);
-} else {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _(sgio of shared disk '%s' conflicts with other 
- active domains), disk-src-path);
-}
-
+   sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED)))
 goto cleanup;
-}
 
 ret = 0;
 
@@ -1088,6 +1073,47 @@ qemuCheckSharedDisk(virHashTablePtr sharedDevices,
 }
 
 
+/* Check if a shared device's setting conflicts with the conf
+ * used by other domain(s). Currently only checks the sgio
+ * setting. Note that this should only be called for disk with
+ * block source if the device type is disk.
+ *
+ * Returns 0 if no conflicts, otherwise returns -1.
+ */
+static int
+qemuCheckSharedDisk(virHashTablePtr sharedDevices,
+virDomainDiskDefPtr disk)
+{
+int ret = -1;
+
+if (disk-device != VIR_DOMAIN_DISK_DEVICE_LUN)
+return 0;
+
+if (virCheckUnprivSGIO(sharedDevices, disk-src-path, disk-sgio)  0) {
+if (virGetLastError() == NULL) {
+if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_VOLUME) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _(sgio of shared disk 'pool=%s' 'volume=%s' 
+ conflicts with other active domains),
+   disk-src-srcpool-pool,
+   disk-src-srcpool-volume);
+} else {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _(sgio of shared disk '%s' conflicts with 
+ other active domains),
+   disk-src-path);
+}
+}
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+return ret;
+}
+
+
 bool
 qemuSharedDeviceEntryDomainExists(qemuSharedDeviceEntryPtr entry,
   const char *name,
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com

[libvirt] [PATCH v2 02/10] qemu: Introduce qemuIsSharedHostdev

2015-07-06 Thread John Ferlan
Add a single boolean function to handle whether the hostdev is shared or not.

Use the new function for the qemu{Add|Remove}SharedHostdev calls as well
as qemuSetUnprivSGIO. NB: This second usage fixes a possible bug where
if this feature is enabled at some time in the future and the shareable flag
wasn't set, the sgio would have been erroneously set.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_conf.c | 30 ++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d521886..48fb74a 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1201,6 +1201,19 @@ qemuAddSharedDisk(virQEMUDriverPtr driver,
 }
 
 
+static bool
+qemuIsSharedHostdev(virDomainHostdevDefPtr hostdev)
+{
+if (hostdev-shareable 
+(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS 
+ hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI 
+ hostdev-source.subsys.u.scsi.protocol !=
+ VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI))
+return true;
+return false;
+}
+
+
 static char *
 qemuGetSharedHostdevKey(virDomainHostdevDefPtr hostdev)
 {
@@ -1238,10 +1251,7 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
 char *key = NULL;
 int ret = -1;
 
-if (!hostdev-shareable ||
-!(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS 
-  hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI 
-  hostdev-source.subsys.u.scsi.protocol != 
VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI))
+if (!qemuIsSharedHostdev(hostdev))
 return 0;
 
 if (!(key = qemuGetSharedHostdevKey(hostdev)))
@@ -1342,10 +1352,7 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver,
 char *key = NULL;
 int ret;
 
-if (!hostdev-shareable ||
-!(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS 
-  hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI 
-  hostdev-source.subsys.u.scsi.protocol != 
VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI))
+if (!qemuIsSharedHostdev(hostdev))
 return 0;
 
 if (!(key = qemuGetSharedHostdevKey(hostdev)))
@@ -1407,11 +1414,10 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
 } else if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) {
 hostdev = dev-data.hostdev;
 
+if (!qemuIsSharedHostdev(hostdev))
+return 0;
 
-if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS 
-hostdev-source.subsys.type ==
-VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI 
-hostdev-source.subsys.u.scsi.sgio) {
+if (hostdev-source.subsys.u.scsi.sgio) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_('sgio' is not supported for SCSI 
  generic device yet ));
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 10/10] qemu: Add check for unpriv sgio for SCSI generic host device

2015-07-06 Thread John Ferlan
Check if the hostdev has set the sgio filtered/unfiltered and handle
appropriately.

This restores functionality removed by commit id 'ce346623' to remove
sgio support for the SCSI generic host device.

For most kernels the result of this operation is a no-op; however, for
those that do support it the check is necessary.

Note that this patch fixes a bug found in the reverted change where
if the virGetDeviceUnprivSGIO returned either 0 or 1 in 'val', the
'disk' would be dereferenced; however, since a hostdev didn't have
one - that dereference would have caused a segfault. Instead these
changes use the hostdev's address value.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_conf.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 7df971b..7b45a16 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1281,6 +1281,8 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
 {
 char *dev_path = NULL;
 char *key = NULL;
+virDomainHostdevSubsysSCSIPtr scsisrc = hostdev-source.subsys.u.scsi;
+virDomainHostdevSubsysSCSIHostPtr scsihostsrc = scsisrc-u.host;
 int ret = -1;
 
 if (!qemuIsSharedHostdev(hostdev))
@@ -1291,6 +1293,18 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
 if (!(dev_path = qemuGetHostdevPath(hostdev)))
 goto cleanup;
 
+if (virCheckUnprivSGIO(driver-sharedDevices, dev_path,
+   scsisrc-sgio)  0) {
+if (virGetLastError() == NULL) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _(sgio of shared scsi host device '%s-%u-%u-%llu' 
+ conflicts with other active domains),
+   scsihostsrc-adapter, scsihostsrc-bus,
+   scsihostsrc-target, scsihostsrc-unit);
+}
+goto cleanup;
+}
+
 if (!(key = qemuGetSharedHostdevKey(dev_path)))
 goto cleanup;
 
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 05/10] qemu: Refactor qemuAddSharedHostdev and qemuRemoveSharedHostdev

2015-07-06 Thread John Ferlan
Refactor the functions to follow logic from qemuAddSharedDisk and
qemuRemoveSharedDisk with respect to locking driver.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_conf.c | 27 ++-
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index eb0b34f..8afbddc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1291,13 +1291,18 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
 if (!qemuIsSharedHostdev(hostdev))
 return 0;
 
+qemuDriverLock(driver);
+
 if (!(key = qemuGetSharedHostdevKey(hostdev)))
-return -1;
+goto cleanup;
 
-qemuDriverLock(driver);
-ret = qemuSharedDeviceEntryInsert(driver, key, name);
-qemuDriverUnlock(driver);
+if (qemuSharedDeviceEntryInsert(driver, key, name)  0)
+goto cleanup;
+
+ret = 0;
 
+ cleanup:
+qemuDriverUnlock(driver);
 VIR_FREE(key);
 return ret;
 }
@@ -1387,18 +1392,22 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver,
 const char *name)
 {
 char *key = NULL;
-int ret;
+int ret = -1;
 
 if (!qemuIsSharedHostdev(hostdev))
 return 0;
 
+qemuDriverLock(driver);
+
 if (!(key = qemuGetSharedHostdevKey(hostdev)))
-return -1;
+goto cleanup;
 
-qemuDriverLock(driver);
-ret = qemuSharedDeviceEntryRemove(driver, key, name);
-qemuDriverUnlock(driver);
+if (qemuSharedDeviceEntryRemove(driver, key, name)  0)
+goto cleanup;
 
+ret = 0;
+ cleanup:
+qemuDriverUnlock(driver);
 VIR_FREE(key);
 return ret;
 }
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-glib PATCHv4 7/7] gobject: Add wrapper for virNetworkGetDHCPLeases

2015-07-06 Thread Zeeshan Ali (Khattak)
On Mon, Jul 6, 2015 at 6:05 PM, Christophe Fergeau cferg...@redhat.com wrote:
 On Wed, Jul 01, 2015 at 09:40:51PM +0100, Zeeshan Ali (Khattak) wrote:
 ---
  libvirt-gobject/libvirt-gobject-network.c | 54 
 +++
  libvirt-gobject/libvirt-gobject-network.h |  4 +++
  libvirt-gobject/libvirt-gobject.sym   |  2 ++
  3 files changed, 60 insertions(+)

 diff --git a/libvirt-gobject/libvirt-gobject-network.c 
 b/libvirt-gobject/libvirt-gobject-network.c
 index b1b38a0..b29be36 100644
 --- a/libvirt-gobject/libvirt-gobject-network.c
 +++ b/libvirt-gobject/libvirt-gobject-network.c
 @@ -29,6 +29,7 @@
  #include libvirt-glib/libvirt-glib.h
  #include libvirt-gobject/libvirt-gobject.h
  #include libvirt-gobject-compat.h
 +#include libvirt-gobject/libvirt-gobject-network-dhcp-lease-private.h

  #define GVIR_NETWORK_GET_PRIVATE(obj) \
  (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_NETWORK, 
 GVirNetworkPrivate))
 @@ -224,3 +225,56 @@ GVirConfigNetwork *gvir_network_get_config(GVirNetwork 
 *network,
  free(xml);
  return conf;
  }
 +
 +/**
 + * gvir_network_get_dhcp_leases:
 + * @network: the network
 + * @mac: (allow-none): The optional ASCII formatted MAC address of an 
 interface
 + * @flags: placeholder for flags, pass 0

 must be 0 rather than pass 0 ?

I just copypasted from another place in code. :)

-- 
Regards,

Zeeshan Ali (Khattak)

Befriend GNOME: http://www.gnome.org/friends/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-glib PATCHv4 6/7] gobject: Add wrapper for virNetworkDHCPLease

2015-07-06 Thread Christophe Fergeau
On Wed, Jul 01, 2015 at 09:40:50PM +0100, Zeeshan Ali (Khattak) wrote:
 diff --git a/libvirt-gobject/libvirt-gobject.sym 
 b/libvirt-gobject/libvirt-gobject.sym
 index 88ca271..d345813 100644
 --- a/libvirt-gobject/libvirt-gobject.sym
 +++ b/libvirt-gobject/libvirt-gobject.sym
 @@ -287,6 +287,19 @@ LIBVIRT_GOBJECT_0.2.2 {
   gvir_connection_get_networks;
  
   gvir_interface_get_mac;
 +
 + gvir_ip_addr_type_get_type;
 +
 + gvir_network_dhcp_lease_get_clientid;
 + gvir_network_dhcp_lease_get_expirytime;
 + gvir_network_dhcp_lease_get_hostname;
 + gvir_network_dhcp_lease_get_iaid;
 + gvir_network_dhcp_lease_get_iface;
 + gvir_network_dhcp_lease_get_ip;
 + gvir_network_dhcp_lease_get_ip_type;
 + gvir_network_dhcp_lease_get_mac;
 + gvir_network_dhcp_lease_get_prefix;
 + gvir_network_dhcp_lease_get_type;

What about the second paragraph from
https://www.redhat.com/archives/libvir-list/2015-June/msg01649.html:
« [snip]
Just realized now, but wouldn't we be better off with
gvir_network_dhcp_lease_get_client_id;
gvir_network_dhcp_lease_get_expiry_time;

instead of

gvir_network_dhcp_lease_get_clientid;
gvir_network_dhcp_lease_get_expirytime; »

Christophe


pgpNkf3B0MZo2.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] vz: assign static IPs and default gateways for network adapter

2015-07-06 Thread Dmitry Guryanov

On 07/01/2015 01:15 PM, Mikhail Feoktistov wrote:

We support only one IPv4 and one IPv6 default gateway.
If static IPs are not present in instance config,
then we switch on DHCP for this adapter.
PrlVmDevNet_SetAutoApply to makes necessary settings within guest OS
In linux case it creates network startup scripts
/etc/sysconfig/network-scripts/ifcfg-ethN and fills it with necessary
parameters.
---
  src/vz/vz_sdk.c |  120 +++
  1 files changed, 120 insertions(+), 0 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f27098c..7d318f8 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2788,8 +2788,13 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
  PRL_HANDLE sdknet = PRL_INVALID_HANDLE;
  PRL_HANDLE vnet = PRL_INVALID_HANDLE;
  PRL_HANDLE job = PRL_INVALID_HANDLE;
+PRL_HANDLE addrlist = PRL_INVALID_HANDLE;
+size_t i;
  int ret = -1;
  char macstr[PRL_MAC_STRING_BUFNAME];
+char *addrstr = NULL;
+bool ipv6present = false;
+bool ipv4present = false;
  
  if (prlsdkCheckNetUnsupportedParams(net)  0)

  return -1;
@@ -2814,6 +2819,118 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
  pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
  prlsdkCheckRetGoto(pret, cleanup);
  
+pret = PrlApi_CreateStringsList(addrlist);

+prlsdkCheckRetGoto(pret, cleanup);
+
+for (i = 0; i  net-nips; i++) {
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char *tmpstr;
+
+if (AF_INET == VIR_SOCKET_ADDR_FAMILY(net-ips[i]-address))
+ipv4present = true;
+else if (AF_INET6 == VIR_SOCKET_ADDR_FAMILY(net-ips[i]-address))
+ipv6present = true;
+else
+continue;
+
+if (!(tmpstr = virSocketAddrFormat(net-ips[i]-address)))
+goto cleanup;
+
+virBufferAsprintf(buf, %s/%d, tmpstr, net-ips[i]-prefix);
+VIR_FREE(tmpstr);
+if (!(addrstr = virBufferContentAndReset(buf)))
+goto cleanup;


It's better to use virAsprintf here, since you do string formatting only 
once.



+
+pret = PrlStrList_AddItem(addrlist, addrstr);
+prlsdkCheckRetGoto(pret, cleanup);
+
+VIR_FREE(addrstr);
+addrstr = NULL;
VIR_FREE sets the pointer to NULL, you don't need to do it by yourself. 
Also it's being caught by make syntax-check Could you, please, run it 
before sending patches?




+}
+
+if (ipv4present || ipv6present) {
+pret = PrlVmDevNet_SetNetAddresses(sdknet, addrlist);
+prlsdkCheckRetGoto(pret, cleanup);
+}
+
+pret = PrlVmDevNet_SetConfigureWithDhcp(sdknet, !ipv4present);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVmDevNet_SetConfigureWithDhcpIPv6(sdknet, !ipv6present);
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlVmDevNet_SetAutoApply(sdknet, true);
+prlsdkCheckRetGoto(pret, cleanup);
+
+if (net-nroutes) {


As I can remember, it's not possible to set gateway together with DHCP. 
So I think you should set up gateways before dhcp and only if 
ipv4present/ipv6present is true.



+bool alreadySetIPv4Gateway = false;
+bool alreadySetIPv6Gateway = false;
+
+for (i = 0; i  net-nroutes; i++) {
+virSocketAddrPtr addrdst, gateway;
+virSocketAddr zero;
+
+addrdst = virNetworkRouteDefGetAddress(net-routes[i]);
+gateway = virNetworkRouteDefGetGateway(net-routes[i]);
+
+ignore_value(virSocketAddrParse(zero,
+(VIR_SOCKET_ADDR_IS_FAMILY(addrdst, 
AF_INET)
+ ? VIR_SOCKET_ADDR_IPV4_ALL
+ : VIR_SOCKET_ADDR_IPV6_ALL),
+VIR_SOCKET_ADDR_FAMILY(addrdst)));
+
+if (!virSocketAddrEqual(addrdst, zero)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Support only default gateway));
+goto cleanup;
+}
+
+switch (VIR_SOCKET_ADDR_FAMILY(gateway)) {
+case AF_INET:
+if (alreadySetIPv4Gateway) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Support only one IPv4 default gateway));
+goto cleanup;
+}
+
+if (!(addrstr = virSocketAddrFormat(gateway)))
+goto cleanup;
+
+pret = PrlVmDevNet_SetDefaultGateway(sdknet, addrstr);
+prlsdkCheckRetGoto(pret, cleanup);
+
+alreadySetIPv4Gateway = true;
+break;
+
+case AF_INET6:
+if (alreadySetIPv6Gateway) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Support only one IPv6 default gateway));
+goto cleanup;
+}
+
+if (!(addrstr = 

[libvirt] [PATCH] vz: use PRL_USE_VNET_NAME_FOR_BRIDGE_NAME

2015-07-06 Thread Maxim Nestratov
It is better not to assume that newly created network should be
connected to a bridge with same name, but specify it explicitly
by PRL_USE_VNET_NAME_FOR_BRIDGE_NAME flag.

Signed-off-by: Maxim Nestratov mnestra...@virtuozzo.com
---
 src/vz/vz_sdk.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 1b66958..1c56655 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2834,7 +2834,9 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
 pret = PrlVirtNet_SetNetworkType(vnet, PVN_BRIDGED_ETHERNET);
 prlsdkCheckRetGoto(pret, cleanup);
 
-job = PrlSrv_AddVirtualNetwork(privconn-server, vnet, 0);
+job = PrlSrv_AddVirtualNetwork(privconn-server,
+   vnet,
+   PRL_USE_VNET_NAME_FOR_BRIDGE_NAME);
 if (PRL_FAILED(pret = waitJob(job)))
 goto cleanup;
 
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] libxl: support dom0

2015-07-06 Thread Jim Fehlig
In Xen, dom0 is really just another domain that supports ballooning,
adding/removing devices, changing vcpu configuration, etc. This patch
adds support to the libxl driver for managing dom0. Note that the
legacy xend driver has long supported managing dom0.

Operations that are not supported on dom0 are filtered in libvirt
where a sensible error is reported. Errors from libxl are not
always helpful. E.g., attempting a save on dom0 results in

2015-06-23 15:25:05 MDT libxl: debug: libxl_dom.c:1570:libxl__toolstack_save: 
domain=0 toolstack data size=8
2015-06-23 15:25:05 MDT libxl: debug: libxl.c:979:do_libxl_domain_suspend: ao 
0x7f7e68000b70: inprogress: poller=0x7f7e68000930, flags=i
2015-06-23 15:25:05 MDT libxl-save-helper: debug: starting save: Success
2015-06-23 15:25:05 MDT xc: detail: xc_domain_save_suse: starting save of domid 0
2015-06-23 15:25:05 MDT xc: error: Couldn't map live_shinfo (3 = No such 
process): Internal error
2015-06-23 15:25:05 MDT xc: detail: Save exit of domid 0 with errno=3
2015-06-23 15:25:05 MDT libxl-save-helper: debug: complete r=1: No such process
2015-06-23 15:25:05 MDT libxl: error: 
libxl_dom.c:1876:libxl__xc_domain_save_done: saving domain: domain did not 
respond to suspend request: No such process
2015-06-23 15:25:05 MDT libxl: error: libxl_dom.c:2033:remus_teardown_done: 
Remus: failed to teardown device for guest with domid 0, rc -8

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 95 
 1 file changed, 95 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 149ef70..d0b76ac 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -79,6 +79,15 @@ VIR_LOG_INIT(libxl.libxl_driver);
 /* Number of Xen scheduler parameters */
 #define XEN_SCHED_CREDIT_NPARAM   2
 
+#define LIBXL_CHECK_DOM0_GOTO(name, label)   \
+do {  \
+if (STREQ_NULLABLE(name, Domain-0)) {   \
+virReportError(VIR_ERR_OPERATION_INVALID, %s,   \
+   _(Domain-0 does not support requested 
operation)); \
+goto label;   \
+} \
+} while (0)
+
 
 static libxlDriverPrivatePtr libxl_driver;
 
@@ -501,6 +510,62 @@ const struct libxl_event_hooks ev_hooks = {
 };
 
 static int
+libxlAddDom0(libxlDriverPrivatePtr driver)
+{
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+virDomainDefPtr def = NULL;
+virDomainObjPtr vm = NULL;
+virDomainDefPtr oldDef = NULL;
+libxl_dominfo d_info;
+int ret = -1;
+
+libxl_dominfo_init(d_info);
+
+/* Ensure we have a dom0 */
+if (libxl_domain_info(cfg-ctx, d_info, 0) != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   %s, _(unable to get Domain-0 information from 
libxenlight));
+goto cleanup;
+}
+
+if (!(def = virDomainDefNew()))
+goto cleanup;
+
+def-id = 0;
+def-virtType = VIR_DOMAIN_VIRT_XEN;
+if (VIR_STRDUP(def-name, Domain-0)  0)
+goto cleanup;
+
+def-os.type = VIR_DOMAIN_OSTYPE_XEN;
+
+if (virUUIDParse(----, def-uuid)  0)
+goto cleanup;
+
+vm-def-vcpus = d_info.vcpu_online;
+vm-def-maxvcpus = d_info.vcpu_max_id + 1;
+vm-def-mem.cur_balloon = d_info.current_memkb;
+vm-def-mem.max_balloon = d_info.max_memkb;
+
+if (!(vm = virDomainObjListAdd(driver-domains, def,
+   driver-xmlopt,
+   0,
+   oldDef)))
+goto cleanup;
+
+def = NULL;
+ret = 0;
+
+ cleanup:
+libxl_dominfo_dispose(d_info);
+virDomainDefFree(def);
+virDomainDefFree(oldDef);
+if (vm)
+virObjectUnlock(vm);
+virObjectUnref(cfg);
+return ret;
+}
+
+static int
 libxlStateInitialize(bool privileged,
  virStateInhibitCallback callback ATTRIBUTE_UNUSED,
  void *opaque ATTRIBUTE_UNUSED)
@@ -616,6 +681,10 @@ libxlStateInitialize(bool privileged,
 if (!(libxl_driver-xmlopt = libxlCreateXMLConf()))
 goto error;
 
+/* Add Domain-0 */
+if (libxlAddDom0(libxl_driver)  0)
+goto error;
+
 /* Load running domains first. */
 if (virDomainObjListLoadAllConfigs(libxl_driver-domains,
cfg-stateDir,
@@ -1030,6 +1099,8 @@ libxlDomainSuspend(virDomainPtr dom)
 if (!(vm = libxlDomObjFromDomain(dom)))
 goto cleanup;
 
+LIBXL_CHECK_DOM0_GOTO(vm-def-name, cleanup);
+
 if (virDomainSuspendEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;
 
@@ -1086,6 +1157,8 @@ libxlDomainResume(virDomainPtr dom)
 if (!(vm = libxlDomObjFromDomain(dom)))
 goto 

Re: [libvirt] [PATCH] libxl: support dom0

2015-07-06 Thread Jim Fehlig

On 07/06/2015 03:46 PM, Jim Fehlig wrote:

In Xen, dom0 is really just another domain that supports ballooning,
adding/removing devices, changing vcpu configuration, etc. This patch
adds support to the libxl driver for managing dom0. Note that the
legacy xend driver has long supported managing dom0.

Operations that are not supported on dom0 are filtered in libvirt
where a sensible error is reported. Errors from libxl are not
always helpful. E.g., attempting a save on dom0 results in

2015-06-23 15:25:05 MDT libxl: debug: libxl_dom.c:1570:libxl__toolstack_save: 
domain=0 toolstack data size=8
2015-06-23 15:25:05 MDT libxl: debug: libxl.c:979:do_libxl_domain_suspend: ao 
0x7f7e68000b70: inprogress: poller=0x7f7e68000930, flags=i
2015-06-23 15:25:05 MDT libxl-save-helper: debug: starting save: Success
2015-06-23 15:25:05 MDT xc: detail: xc_domain_save_suse: starting save of domid 0
2015-06-23 15:25:05 MDT xc: error: Couldn't map live_shinfo (3 = No such 
process): Internal error
2015-06-23 15:25:05 MDT xc: detail: Save exit of domid 0 with errno=3
2015-06-23 15:25:05 MDT libxl-save-helper: debug: complete r=1: No such process
2015-06-23 15:25:05 MDT libxl: error: 
libxl_dom.c:1876:libxl__xc_domain_save_done: saving domain: domain did not 
respond to suspend request: No such process
2015-06-23 15:25:05 MDT libxl: error: libxl_dom.c:2033:remus_teardown_done: 
Remus: failed to teardown device for guest with domid 0, rc -8

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
  src/libxl/libxl_driver.c | 95 
  1 file changed, 95 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 149ef70..d0b76ac 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -79,6 +79,15 @@ VIR_LOG_INIT(libxl.libxl_driver);
  /* Number of Xen scheduler parameters */
  #define XEN_SCHED_CREDIT_NPARAM   2
  
+#define LIBXL_CHECK_DOM0_GOTO(name, label)   \

+do {  \
+if (STREQ_NULLABLE(name, Domain-0)) {   \
+virReportError(VIR_ERR_OPERATION_INVALID, %s,   \
+   _(Domain-0 does not support requested 
operation)); \
+goto label;   \
+} \
+} while (0)
+
  
  static libxlDriverPrivatePtr libxl_driver;
  
@@ -501,6 +510,62 @@ const struct libxl_event_hooks ev_hooks = {

  };
  
  static int

+libxlAddDom0(libxlDriverPrivatePtr driver)
+{
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+virDomainDefPtr def = NULL;
+virDomainObjPtr vm = NULL;
+virDomainDefPtr oldDef = NULL;
+libxl_dominfo d_info;
+int ret = -1;
+
+libxl_dominfo_init(d_info);
+
+/* Ensure we have a dom0 */
+if (libxl_domain_info(cfg-ctx, d_info, 0) != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   %s, _(unable to get Domain-0 information from 
libxenlight));
+goto cleanup;
+}
+
+if (!(def = virDomainDefNew()))
+goto cleanup;
+
+def-id = 0;
+def-virtType = VIR_DOMAIN_VIRT_XEN;
+if (VIR_STRDUP(def-name, Domain-0)  0)
+goto cleanup;
+
+def-os.type = VIR_DOMAIN_OSTYPE_XEN;
+
+if (virUUIDParse(----, def-uuid)  0)
+goto cleanup;
+
+vm-def-vcpus = d_info.vcpu_online;
+vm-def-maxvcpus = d_info.vcpu_max_id + 1;
+vm-def-mem.cur_balloon = d_info.current_memkb;
+vm-def-mem.max_balloon = d_info.max_memkb;


Opps. Before sending the patch, but after testing it again, I moved the call to 
libxl_domain_info to the beginning of this function.  I also moved setting the 
vcpu and memory info earlier, but



+
+if (!(vm = virDomainObjListAdd(driver-domains, def,
+   driver-xmlopt,
+   0,
+   oldDef)))
+goto cleanup;
+
+def = NULL;
+ret = 0;


before getting a virDomainObj - ouch.  Consider the following obvious fix 
squashed in


diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d0b76ac..c0dd00b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -541,18 +541,19 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
 if (virUUIDParse(----, def-uuid)  0)
 goto cleanup;

+if (!(vm = virDomainObjListAdd(driver-domains, def,
+   driver-xmlopt,
+   0,
+   oldDef)))
+goto cleanup;
+
+def = NULL;
+
 vm-def-vcpus = d_info.vcpu_online;
 vm-def-maxvcpus = d_info.vcpu_max_id + 1;
 vm-def-mem.cur_balloon = d_info.current_memkb;
 vm-def-mem.max_balloon = d_info.max_memkb;

-if (!(vm = 

Re: [libvirt] [PATCH v3] nodeinfo: fix to parse present cpus rather than possible cpus

2015-07-06 Thread John Ferlan


On 06/26/2015 06:27 PM, Kothapally Madhu Pavan wrote:
 Currently we are parsing all the possible cpus to get the
 nodeinfo. This fix will perform a check for present cpus
 before parsing.
 
 Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
 
 
 ---
  src/nodeinfo.c |   11 +++
  1 file changed, 11 insertions(+)
 

What you described in your response regarding the directory structure
would be a lot easier to visualize if you modified the nodeinfotest in
order to provide an example. Of course as I found out during this pass
- that won't necessarily do what is expected (yet) either.

Also looks like I mistyped the virBitmapIsBitSet as virBitmapIsSet when
I responded to your prior patch - it was freehand.

So even changing that in this patch, there's an issue with this patch
and existing tests.  Did you run 'make check'?  Did it succeed?  Mine
failed with your patch, but that's partially because I have a 2 CPU 2
core laptop and any nodeinfotest needing data from cpu5 and higher will
fail. Perhaps that may have succeeded if nodeinfo.c was able to handle
non root based paths (e.g. an update to all code paths to pass a path
and if NULL, use SYSFS_SYSTEM_PATH else use the passed path).

If you look for the tests/*nodeinfo*/linux-test*/cpu/present files,
you will note only one exists... and since it's presence is essentially
the basis for your check, a modification of the test is necessary in
order to mimic the environment since right now it will use whatever is
on the host.

That's a fairly invasive change to the nodeinfo.c - something I started
to see if it's possible. Once in place, it'll be much easier for you to
add a test to exhibit the issue you're trying to fix/resolve.

John


 diff --git a/src/nodeinfo.c b/src/nodeinfo.c
 index 2fafe2d..5689c9b 100644
 --- a/src/nodeinfo.c
 +++ b/src/nodeinfo.c
 @@ -43,6 +43,7 @@
  #include c-ctype.h
  #include viralloc.h
  #include nodeinfopriv.h
 +#include nodeinfo.h
  #include physmem.h
  #include virerror.h
  #include count-one-bits.h
 @@ -418,6 +419,7 @@ virNodeParseNode(const char *node,
  int processors = 0;
  DIR *cpudir = NULL;
  struct dirent *cpudirent = NULL;
 +virBitmapPtr present_cpumap = NULL;
  int sock_max = 0;
  cpu_set_t sock_map;
  int sock;
 @@ -438,12 +440,17 @@ virNodeParseNode(const char *node,
  goto cleanup;
  }
  
 +present_cpumap = nodeGetPresentCPUBitmap();
 +
  /* enumerate sockets in the node */
  CPU_ZERO(sock_map);
  while ((direrr = virDirRead(cpudir, cpudirent, node))  0) {
  if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1)
  continue;
  
 +if (present_cpumap  !(virBitmapIsSet(present_cpumap, cpu)))
 +continue;
 +
  if ((online = virNodeGetCpuValue(node, cpu, online, 1))  0)
  goto cleanup;
  
 @@ -477,6 +484,9 @@ virNodeParseNode(const char *node,
  if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1)
  continue;
  
 +if (present_cpumap  !(virBitmapIsSet(present_cpumap, cpu)))
 +continue;
 +
  if ((online = virNodeGetCpuValue(node, cpu, online, 1))  0)
  goto cleanup;
  
 @@ -537,6 +547,7 @@ virNodeParseNode(const char *node,
  ret = -1;
  }
  VIR_FREE(core_maps);
 +virBitmapFree(present_cpumap);
  
  return ret;
  }
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-07-06 Thread Prerna
On Sun, Jul 5, 2015 at 5:13 PM, Qiaowei Ren qiaowei@intel.com wrote:

 One RFC in
 https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html

 CMT (Cache Monitoring Technology) can be used to measure the
 usage of cache by VM running on the host. This patch will
 extend the bulk stats API (virDomainListGetStats) to add this
 field. Applications based on libvirt can use this API to achieve
 cache usage of VM. Because CMT implementation in Linux kernel
 is based on perf mechanism, this patch will enable perf event
 for CMT when VM is created and disable it when VM is destroyed.


Hi Ren,
One query wrt this implementation. I see you make a perf ioctl to gather
CMT stats each time the stats API is invoked.
If the CMT stats are exposed by a hardware counter, then this implies
logging on a per-cpu (or per-socket ???) basis.
This also implies that the value read will vary as the CPU (or socket) on
which it is being called changes.

Now, with this background, if we need real-world stats on a VM, we need
this perf ioctl executed on all CPUs/ sockets on which the VM ran. Also,
once done, we will need to aggregate results from each of these sources.

In this implementation, I am missing this -- there seems no control over
which physical CPU the libvirt worker thread will run and collect the perf
data from. Data collected from this implementation might not accurately
model the system state.
I _think_ libvirt currently has no way of directing a worker thread to
collect stats from a given CPU -- if we do, I would be happy to learn about
it :)

Regards,
Prerna
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list