Re: [libvirt] Missing libxl_device_nic settings

2015-02-02 Thread Jim Fehlig
Kim Larry wrote:
 Then how do I set ip address when a VM has to use bridged network with
 specific ip?

As you describe, and as your patch attempts to do :).  But you'll need a
V2 to address my comments, particularly the need to rebase against
master.  Cedric provided some hints on doing that.

Regards,
Jim

 Clearly libxl supports it, but I don't see any way to set it though
 libvirt.

 JIhoon

 Jim Fehlig jfeh...@suse.com wrote:

 Kim Larry wrote:
 Hi,
 I was trying to pass ip address to scripts/vif-bridge by putting ip
 address=/ in guest config xml file, however, I found that
 libxlMakeNic(which located in libxl/libxl_conf.c:956) doesn't set
 x_nic-ip. So I patched myself but I'm not so sure
 about VIR_DOMAIN_NET_TYPE_ETHERNET. It seems like vif-route, correct?

 If you want to use a routed setup, consider a 'network' type interface.
 E.g.

 interface type='network'
 source network='routed-network'/
 /interface

 where 'routed-network' is a libvirt network with |forward
 mode='route'/. For more details on libvirt networking see the wiki|
 ||
 |http://wiki.libvirt.org/page/Networking|
 http://wiki.libvirt.org/page/Networking%7C
 ||
 ||
 Here is my patch:
 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
 index 0555b91..0effc59 100644
 --- a/src/libxl/libxl_conf.c
 +++ b/src/libxl/libxl_conf.c
 @@ -1047,10 +1047,18 @@ libxlMakeNic(virDomainDefPtr def,
 if (VIR_STRDUP(x_nic-bridge,
 virDomainNetGetActualBridgeName(l_nic))  0)
 return -1;
 - /* fallthrough */
 + if (VIR_STRDUP(x_nic-script, l_nic-script)  0)
 + return -1;
 + if (VIR_STRDUP(x_nic-ip, l_nic-data.bridge.ipaddr)  0)

 You will need to rebase against latest git master. ipaddr was removed
 by commit aa2cc721.

 + return -1;
 + break;
 case VIR_DOMAIN_NET_TYPE_ETHERNET:
 if (VIR_STRDUP(x_nic-script, l_nic-script)  0)
 return -1;
 + if (VIR_STRDUP(x_nic-ip, l_nic-data.ethernet.ipaddr)  0)
 + return -1;
 + if (VIR_STRDUP(x_nic-gatewaydev,
 l_nic-data.ethernet.dev)  0)

 I don't think the last part is right. data.ethernet.dev is the vdev
 name, not a gateway.

 Regards,
 Jim


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


Re: [libvirt] [PATCH v2] Make tests independant of system page size

2015-02-02 Thread Eric Blake
On 02/02/2015 09:19 AM, Daniel P. Berrange wrote:
 Some code paths have special logic depending on the page size
 reported by sysconf, which in turn affects the test results.
 We must mock this so tests always have a consistent page size.
 ---
  cfg.mk   |  8 
  src/libvirt_private.syms |  2 ++
  src/openvz/openvz_util.c |  5 +
  src/qemu/qemu_command.c  |  6 +++---
  src/qemu/qemu_driver.c   |  6 +-
  src/util/virfile.c   |  2 +-
  src/util/virnuma.c   |  6 +++---
  src/util/virutil.c   | 13 +
  src/util/virutil.h   |  3 +++
  src/xen/xen_hypervisor.c |  4 ++--
  tests/qemuxml2argvmock.c |  8 
  tests/qemuxml2argvtest.c |  1 +
  12 files changed, 46 insertions(+), 18 deletions(-)
 

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] virnetdev: fix some issues found by coverity and mingw builds

2015-02-02 Thread Eric Blake
On 02/02/2015 11:31 AM, Pavel Hrdina wrote:
 Commit e562a61a introduced new function to get/set interface state but
 there was misuse of ATTRIBUTE_NONNULL on non-pointer attributes and also
 we need to wrap that functions by #ifdef to not break mingw build.
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---

 +++ b/src/util/virnetdev.c
 @@ -671,12 +671,23 @@ int virNetDevSetIFFlag(const char *ifname,
   *
   * Returns 0 in case of success or -1 on error.
   */
 +#if defined(SIOCSIFFLAGS)  defined(HAVE_STRUCT_IFREQ)
  int virNetDevSetOnline(const char *ifname,
 bool online)
  {
  
  return virNetDevSetIFFlag(ifname, IFF_UP, online);
  }

This feels like it should be safe, except maybe for the fact that it
uses IFF_UP.  Maybe we should wrap that under VIR_IFF_UP (defined to
IFF_UP when the #ifdefs are right, and to 0 otherwise), so that _only_
virNetDevSetIFFlag() has to have a counterpart definition, instead of
repeating lots of #ifdefs on all the callers.

 +++ b/src/util/virnetdev.h
 @@ -201,25 +201,23 @@ int virNetDevDelMulti(const char *ifname,
  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
  
  int virNetDevSetIFFlag(const char *ifname, int flag, bool val)
 -ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
 -ATTRIBUTE_RETURN_CHECK;
 +ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;

Ack to the changes in this file, although I still think the virnetdev.c
changes can be made shorter.

-- 
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 1/2] libxl: fix fd and timer event handling

2015-02-02 Thread Jim Fehlig
Long ago I incorrectly associated libxl fd and timer registrations
with per-domain libxl_ctx objects.  When creating a libxlDomainObjPrivate,
a libxl_ctx is allocated, and libxl_osevent_register_hooks is called
passing a pointer to the libxlDomainObjPrivate.  When an fd or timer
registration occurred, the registration callback received the
libxlDomainObjPrivate, containing the per-domain libxl_ctx.  This
libxl_ctx was then used when informing libxl about fd events or
timer expirations.

The problem with this approach is that fd and timer registrations do not
share the same lifespan as libxlDomainObjPrivate, and hence the per-domain
libxl_ctx ojects.  The result is races between per-domain libxl_ctx's being
destoryed and events firing on associated fds/timers, typically manifesting
as an assert in libxl

libxl_internal.h:2788: libxl__ctx_unlock: Assertion `!r' failed

There is no need to associate libxlDomainObjPrivate objects with libxl's
desire to use libvirt's event loop.  Instead, the driver-wide libxl_ctx can
be used for the fd and timer registrations.

This patch moves the fd and timer handling code away from the
domain-specific code in libxl_domain.c into libxl_driver.c.  While at it,
function names were changed a bit to better describe their purpose.

The unnecessary locking was also removed since the code simply provides a
wrapper over the event loop interface.  Indeed the locks may have been
causing some deadlocks when repeatedly creating/destroying muliple domains.
There have also been rumors about such deadlocks during parallel OpenStack
Tempest runs.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_domain.c | 234 +--
 src/libxl/libxl_driver.c | 201 +++-
 2 files changed, 201 insertions(+), 234 deletions(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 856cfb4..c44799b 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1,7 +1,7 @@
 /*
  * libxl_domain.c: libxl domain object private state
  *
- * Copyright (C) 2011-2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ * Copyright (C) 2011-2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -46,16 +46,6 @@ VIR_ENUM_IMPL(libxlDomainJob, LIBXL_JOB_LAST,
   modify,
 );
 
-/* Object used to store info related to libxl event registrations */
-typedef struct _libxlEventHookInfo libxlEventHookInfo;
-typedef libxlEventHookInfo *libxlEventHookInfoPtr;
-struct _libxlEventHookInfo {
-libxlEventHookInfoPtr next;
-libxlDomainObjPrivatePtr priv;
-void *xl_priv;
-int id;
-};
-
 static virClassPtr libxlDomainObjPrivateClass;
 
 static void
@@ -75,227 +65,6 @@ libxlDomainObjPrivateOnceInit(void)
 
 VIR_ONCE_GLOBAL_INIT(libxlDomainObjPrivate)
 
-static void
-libxlDomainObjFDEventHookInfoFree(void *obj)
-{
-VIR_FREE(obj);
-}
-
-static void
-libxlDomainObjTimerEventHookInfoFree(void *obj)
-{
-libxlEventHookInfoPtr info = obj;
-
-/* Drop reference on libxlDomainObjPrivate */
-virObjectUnref(info-priv);
-VIR_FREE(info);
-}
-
-static void
-libxlDomainObjFDEventCallback(int watch ATTRIBUTE_UNUSED,
-  int fd,
-  int vir_events,
-  void *fd_info)
-{
-libxlEventHookInfoPtr info = fd_info;
-int events = 0;
-
-virObjectLock(info-priv);
-if (vir_events  VIR_EVENT_HANDLE_READABLE)
-events |= POLLIN;
-if (vir_events  VIR_EVENT_HANDLE_WRITABLE)
-events |= POLLOUT;
-if (vir_events  VIR_EVENT_HANDLE_ERROR)
-events |= POLLERR;
-if (vir_events  VIR_EVENT_HANDLE_HANGUP)
-events |= POLLHUP;
-
-virObjectUnlock(info-priv);
-libxl_osevent_occurred_fd(info-priv-ctx, info-xl_priv, fd, 0, events);
-}
-
-static int
-libxlDomainObjFDRegisterEventHook(void *priv,
-  int fd,
-  void **hndp,
-  short events,
-  void *xl_priv)
-{
-int vir_events = VIR_EVENT_HANDLE_ERROR;
-libxlEventHookInfoPtr info;
-
-if (VIR_ALLOC(info)  0)
-return -1;
-
-info-priv = priv;
-info-xl_priv = xl_priv;
-
-if (events  POLLIN)
-vir_events |= VIR_EVENT_HANDLE_READABLE;
-if (events  POLLOUT)
-vir_events |= VIR_EVENT_HANDLE_WRITABLE;
-
-info-id = virEventAddHandle(fd, vir_events, libxlDomainObjFDEventCallback,
- info, libxlDomainObjFDEventHookInfoFree);
-if (info-id  0) {
-VIR_FREE(info);
-return -1;
-}
-
-*hndp = info;
-
-return 0;
-}
-
-static int
-libxlDomainObjFDModifyEventHook(void *priv ATTRIBUTE_UNUSED,
-int fd ATTRIBUTE_UNUSED,
-void 

[libvirt] [PATCH 0/2] libxl: fix handling of fd and timer registrations

2015-02-02 Thread Jim Fehlig
This small series fixes some assertions we occasionally see in the
libxl driver when running libvirt-TCK.  The assertions were due to
races between destroying per-domain libxl_ctx and receiving fd and
timer callbacks associated with them.  The races are masked by
setting DEBUG loglevel in libvirtd.conf, so often missed by
automated test setups that want DEBUG loglevel.

Patch 1 actually fixes the assertions.  Patch2 fixes a stupid mistake.
See the commit messages for details.

Jim Fehlig (2):
  libxl: fix fd and timer event handling
  libxl: Move setup of child processing code to driver initialization

 src/libxl/libxl_domain.c | 244 +--
 src/libxl/libxl_driver.c | 212 +++-
 2 files changed, 212 insertions(+), 244 deletions(-)

-- 
1.8.4.5

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


[libvirt] [PATCH 2/2] libxl: Move setup of child processing code to driver initialization

2015-02-02 Thread Jim Fehlig
Informing libxl how to handle its child proceses should be done once
during driver initialization, not once for each domain-specific
libxl_ctx object.  The related libxl documentation in
$xen-src/tools/libxl/libxl_event.h even mentions that it is best to
call this at initialisation.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_domain.c | 10 --
 src/libxl/libxl_driver.c | 11 +++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index c44799b..b47c1b0 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -521,14 +521,6 @@ const struct libxl_event_hooks ev_hooks = {
 .disaster = NULL,
 };
 
-static const libxl_childproc_hooks libxl_child_hooks = {
-#ifdef LIBXL_HAVE_SIGCHLD_OWNER_SELECTIVE_REAP
-.chldowner = libxl_sigchld_owner_libxl_always_selective_reap,
-#else
-.chldowner = libxl_sigchld_owner_libxl,
-#endif
-};
-
 int
 libxlDomainObjPrivateInitCtx(virDomainObjPtr vm)
 {
@@ -565,8 +557,6 @@ libxlDomainObjPrivateInitCtx(virDomainObjPtr vm)
 goto cleanup;
 }
 
-libxl_childproc_setmode(priv-ctx, libxl_child_hooks, priv);
-
 ret = 0;
 
  cleanup:
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 00dab98..33d915c 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -473,6 +473,14 @@ static const libxl_osevent_hooks libxl_osevent_callbacks = 
{
 .timeout_deregister = libxlTimeoutDeregisterEventHook,
 };
 
+static const libxl_childproc_hooks libxl_child_hooks = {
+#ifdef LIBXL_HAVE_SIGCHLD_OWNER_SELECTIVE_REAP
+.chldowner = libxl_sigchld_owner_libxl_always_selective_reap,
+#else
+.chldowner = libxl_sigchld_owner_libxl,
+#endif
+};
+
 static int
 libxlStateInitialize(bool privileged,
  virStateInhibitCallback callback ATTRIBUTE_UNUSED,
@@ -521,6 +529,9 @@ libxlStateInitialize(bool privileged,
 /* Register the callbacks providing access to libvirt's event loop */
 libxl_osevent_register_hooks(cfg-ctx, libxl_osevent_callbacks, cfg-ctx);
 
+/* Setup child process handling.  See $xen-src/tools/libxl/libxl_event.h */
+libxl_childproc_setmode(cfg-ctx, libxl_child_hooks, cfg-ctx);
+
 libxl_driver-config = cfg;
 if (virFileMakePath(cfg-stateDir)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-- 
1.8.4.5

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


Re: [libvirt] [PATCH 7/8] qemu: support updating interface with boot index

2015-02-02 Thread John Ferlan


On 01/05/2015 02:29 AM, Wang Rui wrote:
 QEMU supported to set device's boot index online recently(since QEMU 2.2.0).
 This patch implements the interface's boot index update lively.
 
 If PCI address is not specified in the new xml, we can also update boot
 index. If boot order is not specified in the new xml, we take it as canceling
 boot index. So pass value:-1 to qmp command(qom-set) to cancel boot index.
 
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  src/qemu/qemu_hotplug.c | 35 ++-
  1 file changed, 30 insertions(+), 5 deletions(-)
 

So now we're adding the same functionality to a network interface?
Maybe a better description above would help me understand what you're
trying to accomplish.

Perhaps let's get the disk devices to be all set before taking this path
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 5eacfce..4c86370 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -1814,6 +1814,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
  bool needLinkStateChange = false;
  bool needReplaceDevDef = false;
  bool needBandwidthSet = false;
 +bool needBootIndexChange = false;
  int ret = -1;
  
  if (!devslot || !(olddev = *devslot)) {
 @@ -1909,6 +1910,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
  goto cleanup;
  }
  
 +/* If(and olny if) PCI adderss is not specified in the new xml, 
 newdev-info.type

s/If(and olny if)/If (and only if) a
s/adderss/address


Going back a few patches for attach  update and my concern over whether
the XML has an address already or not - this shows why I was concerned.
 If you don't yet have an address you could possibly match something
that you weren't expecting to or not match something...

I didn't look too much deeper here.

BTW: If you're adding network, you'll need to document that.

John
 + * here is NONE. We can copy the old device's PCI address to the new 
 one. We can't
 + * copy olddev-info to newdev-info, because other members in 
 newdev-info(such
 + * as bootIndex) should not be overridden.
 + */
 +if (newdev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
 +newdev-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
 +newdev-info.addr.pci.domain = olddev-info.addr.pci.domain;
 +newdev-info.addr.pci.bus = olddev-info.addr.pci.bus;
 +newdev-info.addr.pci.slot = olddev-info.addr.pci.slot;
 +newdev-info.addr.pci.function = olddev-info.addr.pci.function;
 +}
 +
  /* info: if newdev-info is empty, fill it in from olddev,
   * otherwise verify that it matches - nothing is allowed to
   * change. (There is no helper function to do this, so
 @@ -1945,11 +1959,6 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
 _(cannot modify network rom file));
  goto cleanup;
  }
 -if (olddev-info.bootIndex != newdev-info.bootIndex) {
 -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
 -   _(cannot modify network device boot index setting));
 -goto cleanup;
 -}
  /* (end of device info checks) */
  
  if (STRNEQ_NULLABLE(olddev-filter, newdev-filter) ||
 @@ -2101,6 +2110,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
   virDomainNetGetActualBandwidth(newdev)))
  needBandwidthSet = true;
  
 +if (olddev-info.bootIndex != newdev-info.bootIndex)
 +needBootIndexChange = true;
 +
  /* FINALLY - actually perform the required actions */
  
  if (needReconnect) {
 @@ -2141,6 +2153,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
  goto cleanup;
  }
  
 +if (needBootIndexChange) {
 +/* If boot index is to be changed to 0, we can pass value:-1 to
 +   qmp command(qom-set) to cancel boot index. */
 +if (qemuDomainChangeBootIndex(driver, vm, olddev-info,
 +  newdev-info.bootIndex ?
 +  newdev-info.bootIndex : -1)  0)
 +goto cleanup;
 +/* we successfully switched to the new boot index, and we've
 + * determined that the rest of newdev is equivalent to olddev,
 + * so move newdev into place */
 +needReplaceDevDef = true;
 +}
 +
  if (needReplaceDevDef) {
  /* the changes above warrant replacing olddev with newdev in
   * the domain's nets list.
 

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


Re: [libvirt] [PATCH 2/8] qemu: support attachment of net device with boot index

2015-02-02 Thread John Ferlan


On 01/05/2015 02:29 AM, Wang Rui wrote:
 When we attach an interface to a running VM with boot index, we get a
 successful result. But in fact the boot index won't take effect. QEMU
 supported to set device's boot index online recently(since QEMU 2.2.0).
 
 After this patch, the boot index will take effect after
 virDomainAttachDevice(Flags) API returning success.
 
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  src/qemu/qemu_hotplug.c | 33 +
  src/qemu/qemu_hotplug.h |  4 
  2 files changed, 37 insertions(+)
 
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 7f93b9b..919a061 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -1087,6 +1087,14 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
  /* link set to down */
  }
  
 +if (net-info.bootIndex  0) {
 +if (qemuDomainChangeBootIndex(driver, vm, net-info,
 +  net-info.bootIndex)  0) {
 +virDomainAuditNet(vm, NULL, net, attach, false);

[1] See note below...

 +goto try_remove;
 +}
 +}
 +
  virDomainAuditNet(vm, NULL, net, attach, true);
  
  ret = 0;
 @@ -1853,6 +1861,31 @@ int qemuDomainChangeNetLinkState(virQEMUDriverPtr 
 driver,
  }
  
  int
 +qemuDomainChangeBootIndex(virQEMUDriverPtr driver,
 +  virDomainObjPtr vm,
 +  virDomainDeviceInfoPtr devInfo,
 +  int newBootIndex)
 +{
 +int ret = -1;
 +qemuDomainObjPrivatePtr priv = vm-privateData;
 +
 +if (!devInfo-alias) {
 +virReportError(VIR_ERR_OPERATION_FAILED, %s,
 +   _(can't change boot index: device alias not found));

s/can't/cannot   (I know you copied it from ChangeNetLinkState)

 +return -1;
 +}
 +
 +VIR_DEBUG(Change dev: %s boot index from %d to %d, devInfo-alias,
 +  devInfo-bootIndex, newBootIndex);

Interesting that from patch 1 on you checked !name in the subsequent
SetBootIndex call and failed. Since you make that check above all the
more reason to go with the ATTRIBUTE_NONNULL as I suggested.

 +
 +qemuDomainObjEnterMonitor(driver, vm);
 +ret = qemuMonitorSetBootIndex(priv-mon, devInfo-alias, newBootIndex);
 +qemuDomainObjExitMonitor(driver, vm);

[1] Due to commit id '5c703ca39' a check of the return is required.
However, it seems that this can follow other code from the calling
function and do an 'ignore_value();'.

If ExitMonitor returns  0, then the vm is dead - that could cause
issues in the calling path which makes a virDomainNetAudit() call. Other
error path code from the caller will ignore status and audit, so I
suppose this could follow that, but it may not be as safe as expected.


 +
 +return ret;
 +}
 +
 +int
  qemuDomainChangeNet(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  virDomainPtr dom,
 diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
 index d13c532..3af0875 100644
 --- a/src/qemu/qemu_hotplug.h
 +++ b/src/qemu/qemu_hotplug.h
 @@ -57,6 +57,10 @@ int qemuDomainAttachHostDevice(virConnectPtr conn,
 virDomainHostdevDefPtr hostdev);
  int qemuDomainFindGraphicsIndex(virDomainDefPtr def,
  virDomainGraphicsDefPtr dev);
 +int qemuDomainChangeBootIndex(virQEMUDriverPtr driver,
 +  virDomainObjPtr vm,
 +  virDomainDeviceInfoPtr devInfo,
 +  int newBootIndex);

s/);/)
ATTRIBUTE_NONNULL(3);

especially since you access it without checking for it...

John
  int qemuDomainChangeGraphics(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   virDomainGraphicsDefPtr dev);
 

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


Re: [libvirt] [PATCH] Make tests independant of system page size

2015-02-02 Thread Michal Privoznik
On 02.02.2015 12:10, Daniel P. Berrange wrote:
 Some code paths have special logic depending on the page size
 reported by sysconf, which in turn affects the test results.
 We must mock this so tests always have a consistent page size.
 ---
  src/libvirt_private.syms |  2 ++
  src/openvz/openvz_util.c |  5 +
  src/qemu/qemu_command.c  |  4 ++--
  src/qemu/qemu_driver.c   |  6 +-
  src/util/virfile.c   |  2 +-
  src/util/virnuma.c   |  6 +++---
  src/util/virutil.c   | 13 +
  src/util/virutil.h   |  3 +++
  src/xen/xen_hypervisor.c |  4 ++--
  tests/qemuxml2argvmock.c | 13 +
  tests/qemuxml2argvtest.c |  1 +
  11 files changed, 42 insertions(+), 17 deletions(-)

ACK with this squashed in:

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 791afe5..3b6eddc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4540,7 +4540,7 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 virDomainHugePagePtr master_hugepage = NULL;
 virDomainHugePagePtr hugepage = NULL;
 virDomainNumatuneMemMode mode;
-const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+const long system_page_size = virGetSystemPageSizeKB();
 virMemAccess memAccess = def-cpu-cells[guestNode].memAccess;
 size_t i;
 char *mem_path = NULL;

Should we also introduce a syntax-check rule to prefer
virGetSystemPageSize*()?

Michal

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


Re: [libvirt] [PATCH] util: recheck the validating backend when the firewalld start/stop

2015-02-02 Thread Luyao Huang


On 02/02/2015 07:38 PM, Daniel P. Berrange wrote:

On Mon, Feb 02, 2015 at 11:40:44AM +0800, Luyao Huang wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1188088

When the firewalld is running and then start the libvirtd, libvirt
will set the current backend as VIR_FIREWALL_BACKEND_FIREWALLD.
But when firewalld is stop, we still try to use firewalld even it
is stopped, this will make the vm which has nwfilter cannot start
because systemd cannot find a running firewalld service.

We already have a Dbus callback functions before, add a recheck for
the validating backend in firewalld_dbus_filter_bridge and
nwfilterFirewalldDBusFilter callback functions to help us dynamic
change the validating backend.

NACK, this is not desirable IMHO.  Just because firewalld is stopped
does not imply that it should not be used by libvirt. It may simply
be in the process of being restarted, either by the admin or due to
an RPM upgrade.  Switching a host between firewalld  non-firewalld
managmenet is not something that is typically done - the decision
to use firewalld is something taken at time of initial provisioning.
So I don't think libvirt should optimize for that scenario. We should
optimize for a host always using one or the other exclusively and not
try to dynamically switch.


Got it, i hadn't thought about this when i wrote this patch.

And thanks a lot for your clearly explanation.


Regards,
Daniel


Luyao

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


Re: [libvirt] [PATCH] Make tests independant of system page size

2015-02-02 Thread Martin Kletzander

On Mon, Feb 02, 2015 at 11:10:44AM +, Daniel P. Berrange wrote:

Some code paths have special logic depending on the page size
reported by sysconf, which in turn affects the test results.
We must mock this so tests always have a consistent page size.
---
src/libvirt_private.syms |  2 ++
src/openvz/openvz_util.c |  5 +
src/qemu/qemu_command.c  |  4 ++--
src/qemu/qemu_driver.c   |  6 +-
src/util/virfile.c   |  2 +-
src/util/virnuma.c   |  6 +++---
src/util/virutil.c   | 13 +
src/util/virutil.h   |  3 +++
src/xen/xen_hypervisor.c |  4 ++--
tests/qemuxml2argvmock.c | 13 +
tests/qemuxml2argvtest.c |  1 +
11 files changed, 42 insertions(+), 17 deletions(-)


[...]

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 3037293..f3e8920 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2577,3 +2577,16 @@ virGetListenFDs(void)
}

#endif /* WIN32 */
+
+long virGetSystemPageSize(void)
+{
+return sysconf(_SC_PAGESIZE);
+}
+
+long virGetSystemPageSizeKB(void)
+{
+long val = sysconf(_SC_PAGESIZE);


Use virGetSystemPageSize() here and you don't have to mock 2
functions.

ACK with that changed.


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

Re: [libvirt] [RFC] [Patch] Make hugepage testcase arch-agnostic

2015-02-02 Thread Daniel P. Berrange
On Mon, Feb 02, 2015 at 09:41:33AM +0100, Ján Tomko wrote:
 On Mon, Feb 02, 2015 at 01:16:31PM +0530, Prerna Saxena wrote:
  Hi,
  I have attached this patch as a response to a recent failure observed on 
  PowerPC architecture by commit
  311b4a67.
  This patch introduces a check for dynamically obtaining system page size 
  for test hugepages-pages6 under 'qemuxml2argv' suite. ( See patch for more 
  verbose problem description)
  This patch is not the most perfect implementation -- it fails syntax check; 
  and has a Makefile-driven cleanup pending. I will be happy to deck it up 
  and send it if the community concurs with this
  approach.
  
  We could also implement this via a shell script ( just like 
  'virt-test-aa-helper')  but I couldnt find an easy way to determine host 
  page size.
  
  Awaiting community responses,
  Prerna
  
 
 My preferred solution would be separating all the sysconf(_SC_PAGESIZE)
 calls into some virGetSystemPageSize helper and mocking it in
 qemuxml2argvmock.c, always returning 4 KB.

I've got a change doing exactly that which I'm in middle of testing.

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] virsh-volume: add support for --reflink

2015-02-02 Thread John Ferlan


On 02/01/2015 10:14 PM, Chen Hanxiao wrote:
 add support for --reflink to specify
 VIR_STORAGE_VOL_CREATE_REFLINK flag.
 
 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
  tools/virsh-volume.c | 16 
  1 file changed, 16 insertions(+)
 

What about the man page (virsh.pod) to describe --reflink?


 diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
 index d585ee2..db94154 100644
 --- a/tools/virsh-volume.c
 +++ b/tools/virsh-volume.c
 @@ -204,6 +204,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
  
  if (vshCommandOptBool(cmd, prealloc-metadata))
  flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
 +
  if (!(pool = vshCommandOptPool(ctl, cmd, pool, NULL)))
  return false;
  
 @@ -378,6 +379,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
  
  if (vshCommandOptBool(cmd, prealloc-metadata))
  flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
 +
  if (!(pool = vshCommandOptPool(ctl, cmd, pool, NULL)))
  return false;
  
 @@ -441,6 +443,10 @@ static const vshCmdOptDef opts_vol_create_from[] = {
   .type = VSH_OT_BOOL,
   .help = N_(preallocate metadata (for qcow2 instead of full 
 allocation))
  },
 +{.name = reflink,
 + .type = VSH_OT_BOOL,
 + .help = N_(use btrfs COW lightweight copy)
 +},
  {.name = NULL}
  };
  
 @@ -460,6 +466,9 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
  if (vshCommandOptBool(cmd, prealloc-metadata))
  flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
  
 +if (vshCommandOptBool(cmd, reflink))
 +flags |= VIR_STORAGE_VOL_CREATE_REFLINK;
 +
  if (vshCommandOptStringReq(ctl, cmd, file, from)  0)
  goto cleanup;
  
 @@ -554,6 +563,10 @@ static const vshCmdOptDef opts_vol_clone[] = {
   .type = VSH_OT_BOOL,
   .help = N_(preallocate metadata (for qcow2 instead of full 
 allocation))
  },
 +{.name = reflink,
 + .type = VSH_OT_BOOL,
 + .help = N_(use btrfs COW lightweight copy)
 +},
  {.name = NULL}
  };
  
 @@ -574,6 +587,9 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
  if (vshCommandOptBool(cmd, prealloc-metadata))
  flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
  
 +if (vshCommandOptBool(cmd, reflink))
 +flags |= VIR_STORAGE_VOL_CREATE_REFLINK;
 +
  origpool = virStoragePoolLookupByVolume(origvol);
  if (!origpool) {
  vshError(ctl, %s, _(failed to get parent pool));
 

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


Re: [libvirt] [PATCH] util: recheck the validating backend when the firewalld start/stop

2015-02-02 Thread Daniel P. Berrange
On Mon, Feb 02, 2015 at 11:40:44AM +0800, Luyao Huang wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1188088
 
 When the firewalld is running and then start the libvirtd, libvirt
 will set the current backend as VIR_FIREWALL_BACKEND_FIREWALLD.
 But when firewalld is stop, we still try to use firewalld even it
 is stopped, this will make the vm which has nwfilter cannot start
 because systemd cannot find a running firewalld service.
 
 We already have a Dbus callback functions before, add a recheck for
 the validating backend in firewalld_dbus_filter_bridge and
 nwfilterFirewalldDBusFilter callback functions to help us dynamic
 change the validating backend.

NACK, this is not desirable IMHO.  Just because firewalld is stopped
does not imply that it should not be used by libvirt. It may simply
be in the process of being restarted, either by the admin or due to
an RPM upgrade.  Switching a host between firewalld  non-firewalld
managmenet is not something that is typically done - the decision
to use firewalld is something taken at time of initial provisioning.
So I don't think libvirt should optimize for that scenario. We should
optimize for a host always using one or the other exclusively and not
try to dynamically switch.


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] Make tests independant of system page size

2015-02-02 Thread Daniel P. Berrange
Some code paths have special logic depending on the page size
reported by sysconf, which in turn affects the test results.
We must mock this so tests always have a consistent page size.
---
 src/libvirt_private.syms |  2 ++
 src/openvz/openvz_util.c |  5 +
 src/qemu/qemu_command.c  |  4 ++--
 src/qemu/qemu_driver.c   |  6 +-
 src/util/virfile.c   |  2 +-
 src/util/virnuma.c   |  6 +++---
 src/util/virutil.c   | 13 +
 src/util/virutil.h   |  3 +++
 src/xen/xen_hypervisor.c |  4 ++--
 tests/qemuxml2argvmock.c | 13 +
 tests/qemuxml2argvtest.c |  1 +
 11 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bd7870f..aa68797 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2215,6 +2215,8 @@ virGetListenFDs;
 virGetSCSIHostNameByParentaddr;
 virGetSCSIHostNumber;
 virGetSelfLastChanged;
+virGetSystemPageSize;
+virGetSystemPageSizeKB;
 virGetUnprivSGIOSysfsPath;
 virGetUserCacheDirectory;
 virGetUserConfigDirectory;
diff --git a/src/openvz/openvz_util.c b/src/openvz/openvz_util.c
index 8032f6a..3cdc1c2 100644
--- a/src/openvz/openvz_util.c
+++ b/src/openvz/openvz_util.c
@@ -42,10 +42,7 @@ openvzKBPerPages(void)
 static long kb_per_pages;
 
 if (kb_per_pages == 0) {
-kb_per_pages = sysconf(_SC_PAGESIZE);
-if (kb_per_pages  0) {
-kb_per_pages /= 1024;
-} else {
+if ((kb_per_pages = virGetSystemPageSizeKB())  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Can't determine page size));
 kb_per_pages = 0;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 100deed..30f202f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6694,7 +6694,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 char *nodemask = NULL;
 char *mem_path = NULL;
 int ret = -1;
-const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+const long system_page_size = virGetSystemPageSizeKB();
 
 if (virDomainNumatuneHasPerNodeBinding(def-numatune) 
 !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
@@ -7986,7 +7986,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 def-mem.max_balloon = VIR_DIV_UP(def-mem.max_balloon, 1024) * 1024;
 virCommandAddArgFormat(cmd, %llu, def-mem.max_balloon / 1024);
 if (def-mem.nhugepages  (!def-cpu || !def-cpu-ncells)) {
-const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+const long system_page_size = virGetSystemPageSizeKB();
 char *mem_path = NULL;
 
 if (def-mem.hugepages[0].size == system_page_size) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1d3bee6..0a1c88e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1352,12 +1352,8 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int 
*lastCpu, long *vm_rss,
 if (lastCpu)
 *lastCpu = cpu;
 
-/* We got pages
- * We want kiloBytes
- * _SC_PAGESIZE is page size in Bytes
- * So calculate, but first lower the pagesize so we don't get overflow */
 if (vm_rss)
-*vm_rss = rss * (sysconf(_SC_PAGESIZE)  10);
+*vm_rss = rss * virGetSystemPageSizeKB();
 
 
 VIR_DEBUG(Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld,
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 5f56005..f2b9738 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1086,7 +1086,7 @@ safezero_mmap(int fd, off_t offset, off_t len)
 
 /* align offset and length, rounding offset down and length up */
 if (pagemask == 0)
-pagemask = ~(sysconf(_SC_PAGESIZE) - 1);
+pagemask = ~(virGetSystemPageSize() - 1);
 map_skip = offset - (offset  pagemask);
 
 /* memset wants the mmap'ed file to be present on disk so create a
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 86564d4..e986c71 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -639,7 +639,7 @@ virNumaGetPageInfo(int node,
unsigned int *page_free)
 {
 int ret = -1;
-long system_page_size = sysconf(_SC_PAGESIZE);
+long system_page_size = virGetSystemPageSize();
 
 /* sysconf() returns page size in bytes,
  * the @page_size is however in kibibytes */
@@ -717,7 +717,7 @@ virNumaGetPages(int node,
 
 /* sysconf() returns page size in bytes,
  * but we are storing the page size in kibibytes. */
-system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+system_page_size = virGetSystemPageSizeKB();
 
 /* Query huge pages at first.
  * On Linux systems, the huge pages pool cuts off the available memory and
@@ -841,7 +841,7 @@ virNumaSetPagePoolSize(int node,
 char *end;
 unsigned long long nr_count;
 
-if (page_size == sysconf(_SC_PAGESIZE) / 1024) {
+if (page_size == virGetSystemPageSizeKB()) {
 /* Special case as kernel handles system pages
  * 

Re: [libvirt] [PATCH 1/8] qemu: add a new qemuMonitorSetBootIndex() method to set device's bootorder

2015-02-02 Thread John Ferlan


On 01/05/2015 02:29 AM, Wang Rui wrote:
 The new qemuMonitorSetBootIndex() method can set device' boot order online
 using 'qom-set' JSON monitor command. HMP is not supported. And it is used
 for QEMU = 2.2.0 . The QMP command is like qom-set net1 bootindex 2.
 
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  src/qemu/qemu_monitor.c  | 25 +
  src/qemu/qemu_monitor.h  |  4 
  src/qemu/qemu_monitor_json.c | 19 +++
  src/qemu/qemu_monitor_json.h |  5 +
  4 files changed, 53 insertions(+)
 

Looks like this was missed/lost/forgotten from the holidays...  You'll
need to update with top of tree...

What happens if qemu  2.2?  Is there a way to test?  Callers will end
up failing perhaps when you don't want them to...

After reading later patches - is there a similar qom-get for this field?
 That would simplify a few nasty error paths later..

 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 100bbd0..907834f 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -4292,3 +4292,28 @@ void 
 qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread)
  VIR_FREE(iothread-name);
  VIR_FREE(iothread);
  }
 +
 +int
 +qemuMonitorSetBootIndex(qemuMonitorPtr mon,
 +const char *name,
 +int bootIndex)
 +{
 +int ret;
 +VIR_DEBUG(mon=%p, name=%p:%s, bootIndex=%d, mon, name, name, 
 bootIndex);
 +
 +if (!mon || !name) {
 +virReportError(VIR_ERR_INVALID_ARG, %s,
 +   _(monitor || name must not be NULL));
 +return -1;
 +}

See [1]

 +
 +if (!mon-json) {
 +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
 +   _(JSON monitor is required));
 +return -1;
 +}
 +
 +ret = qemuMonitorJSONSetBootIndex(mon, name, bootIndex);
 +
 +return ret;
 +}
 diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
 index edab66f..8e5d86e 100644
 --- a/src/qemu/qemu_monitor.h
 +++ b/src/qemu/qemu_monitor.h
 @@ -888,6 +888,10 @@ int qemuMonitorGetIOThreads(qemuMonitorPtr mon,
  
  void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread);
  
 +int qemuMonitorSetBootIndex(qemuMonitorPtr mon,
 +const char *name,
 +int bootIndex);

[1]
If you change this slightly to be

   int bootIndex)
   ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

Then you avoid the !mon || !name checks above also the %p:%s becomes
unnecessary - that is either %p or %s.


 +
  /**
   * When running two dd process and using  redirection, we need a
   * shell that will not truncate files.  These two strings serve that
 diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
 index e567aa7..df52101 100644
 --- a/src/qemu/qemu_monitor_json.c
 +++ b/src/qemu/qemu_monitor_json.c
 @@ -6500,3 +6500,22 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
  virJSONValueFree(reply);
  return ret;
  }
 +
 +int
 +qemuMonitorJSONSetBootIndex(qemuMonitorPtr mon,
 +const char *name,
 +int bootIndex)
 +{
 +qemuMonitorJSONObjectProperty prop;
 +
 +memset(prop, 0, sizeof(qemuMonitorJSONObjectProperty));
 +prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
 +prop.val.iv = bootIndex;
 +
 +if (qemuMonitorJSONSetObjectProperty(mon, name,
 + bootindex,
 + prop)  0) {
 +return -1;
 +}
 +return 0;
 +}
 diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
 index 222f11e..6355ddf 100644
 --- a/src/qemu/qemu_monitor_json.h
 +++ b/src/qemu/qemu_monitor_json.h
 @@ -472,4 +472,9 @@ int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr 
 mon);
  int qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
  qemuMonitorIOThreadsInfoPtr **iothreads)
  ATTRIBUTE_NONNULL(2);
 +
 +int qemuMonitorJSONSetBootIndex(qemuMonitorPtr mon,
 +const char *name,
 +int bootIndex);
 +
  #endif /* QEMU_MONITOR_JSON_H */
 

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


Re: [libvirt] [PATCH 5/8] qemu: support attachment of disk device with boot index

2015-02-02 Thread John Ferlan


On 01/05/2015 02:29 AM, Wang Rui wrote:
 When we attach a disk to a running VM with boot index, we can get a
 successful result. But in fact the boot index won't take effect. QEMU
 supported to set device's boot index online recently(since QEMU 2.2.0).
 
 After this patch, the boot index will take effect after
 virDomainAttachDevice(Flags) API returning success. If new disk is
 attached successfully but boot index is set failed, we'll remove the
 new disk to restore.
 
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  src/qemu/qemu_hotplug.c | 88 
 +
  1 file changed, 88 insertions(+)
 
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 2f84949..5eacfce 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -2999,7 +2999,12 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
  {
  virDomainDiskDefPtr disk = dev-data.disk;
  virDomainDiskDefPtr orig_disk = NULL;
 +virDomainDeviceDefPtr new_dev_copy = NULL;
 +virDomainDeviceDefPtr old_dev_copy = NULL;
 +virDomainDiskDefPtr tmp = NULL;
 +virCapsPtr caps = NULL;
  int ret = -1;
 +int removed = 0;
  const char *driverName = virDomainDiskGetDriver(disk);
  const char *src = virDomainDiskGetSource(disk);
  
 @@ -3022,6 +3027,13 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
  if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true)  0)
  goto end;
  
 +if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 +goto end;
 +
 +if (!(new_dev_copy = virDomainDeviceDefCopy(dev, vm-def,
 +caps, driver-xmlopt)))

Format/alignment issue (caps under dev)

 +goto end;
 +
  switch (disk-device)  {
  case VIR_DOMAIN_DISK_DEVICE_CDROM:
  case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
 @@ -3036,12 +3048,33 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
  goto end;
  }
  
 +tmp = dev-data.disk;
 +dev-data.disk = orig_disk;
 +/* save a copy of old device to restore */
 +if (!(old_dev_copy = virDomainDeviceDefCopy(dev, vm-def,
 +caps, driver-xmlopt))) {
 +dev-data.disk = tmp;
 +goto end;
 +}
 +dev-data.disk = tmp;
 +
  if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
 disk-src, false)  0)
  goto end;
  
  disk-src = NULL;
  ret = 0;
 +
 +tmp = new_dev_copy-data.disk;
 +if (orig_disk-info.bootIndex != tmp-info.bootIndex) {
 +/* If boot index is to be changed to 0, we can pass value:-1 to
 +   qmp command(qom-set) to cancel boot index. */
 +if (qemuDomainChangeBootIndex(driver, vm, orig_disk-info,
 +  tmp-info.bootIndex ?
 +  tmp-info.bootIndex : -1)  0)
 +goto try_remove;
 +orig_disk-info.bootIndex = tmp-info.bootIndex;
 +}
  break;
  
  case VIR_DOMAIN_DISK_DEVICE_DISK:
 @@ -3063,6 +3096,15 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
 _(disk bus '%s' cannot be hotplugged.),
 virDomainDiskBusTypeToString(disk-bus));
  }
 +
 +tmp = new_dev_copy-data.disk;
 +if (!ret  tmp-info.bootIndex  0) {
 +if (qemuDomainChangeBootIndex(driver, vm, disk-info,
 +  tmp-info.bootIndex)  0)
 +goto try_remove;
 +disk-info.bootIndex = tmp-info.bootIndex;
 +}
 +
  break;
  default:
  virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
 @@ -3074,7 +3116,53 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
   end:
  if (ret != 0)
  ignore_value(qemuRemoveSharedDevice(driver, dev, vm-def-name));
 +virObjectUnref(caps);
 +virDomainDeviceDefFree(new_dev_copy);
 +virDomainDeviceDefFree(old_dev_copy);
 +if (removed) {
 +dev-data.disk = NULL;
 +ret = -1;
 +}
  return ret;
 +
 + try_remove:
 +if (!virDomainObjIsActive(vm)) {
 +removed = 1;
 +goto end;
 +}
 +
 +switch (new_dev_copy-data.disk-device)  {
 +case VIR_DOMAIN_DISK_DEVICE_CDROM:
 +case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
 +if (qemuAddSharedDevice(driver, old_dev_copy, vm-def-name)  0)
 +break;
 +
 +tmp = old_dev_copy-data.disk;
 +if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, 
 tmp-src, false)  0) {
 +VIR_WARN(Unable to recover original media with target '%s' and 
 path '%s',
 + tmp-dst, tmp-src-path);
 +ignore_value(qemuRemoveSharedDevice(driver, old_dev_copy, 
 vm-def-name));
 +} 

Re: [libvirt] [PATCH 8/8] qemu: support updating disk with boot index

2015-02-02 Thread John Ferlan


On 01/05/2015 02:29 AM, Wang Rui wrote:
 QEMU supported to set device's boot index online recently(since QEMU 2.2.0).
 This patch implements the disk's boot index update lively.

Description needs some more beef... The usage of update lively doesn't
translate well.  I think you're talking about being able to make an
'online update' of a disk's bootIndex.  Essentially this allows us to
change an existing from 2 to 3 perhaps and then add a new 2 - is
that correct?   What happens if you have 1, 2, 3 ... try to change 3 to
2 - will be an error?  The clearing of index in order to remove it needs
to be made clearer on the formatdomain.html.in and/or virsh.pod.

All this needs to be done via XML editing - prone to errors...  Perhaps
consider adding a virsh option... Makes it a whole lot easier to adjust
things rather than attempting to generate XML to modify things. Trying
to figure out the magic incantation required to remove the index
live by only providing some XML to a virsh command can be a challenge.
Sure you understand it and I'm beginning to, but how would one use
this feature is not described anywhere.

John


 
 If boot order is not specified in the new xml, we take it as canceling boot
 index. So pass value:-1 to qmp command(qom-set) to cancel boot index.
 
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  src/qemu/qemu_driver.c | 64 
 +++---
  1 file changed, 55 insertions(+), 9 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 673d8a6..9615fe4 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -7048,6 +7048,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
  {
  virDomainDiskDefPtr disk = dev-data.disk;
  virDomainDiskDefPtr orig_disk = NULL;
 +int oldBootIndex = -1;
  int ret = -1;
  
  if (virStorageTranslateDiskSourcePool(conn, disk)  0)
 @@ -7056,16 +7057,25 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
  if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true)  0)
  goto end;
  
 +if (!(orig_disk = virDomainDiskFindByBusAndDst(vm-def,
 +   disk-bus, disk-dst))) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(No device with bus '%s' and target '%s'),
 +   virDomainDiskBusTypeToString(disk-bus),
 +   disk-dst);
 +goto end;
 +}
 +
  switch (disk-device) {
  case VIR_DOMAIN_DISK_DEVICE_CDROM:
  case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
 -if (!(orig_disk = virDomainDiskFindByBusAndDst(vm-def,
 -   disk-bus, 
 disk-dst))) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(No device with bus '%s' and target '%s'),
 -   virDomainDiskBusTypeToString(disk-bus),
 -   disk-dst);
 -goto end;
 +if (orig_disk-info.bootIndex != disk-info.bootIndex) {
 +oldBootIndex = orig_disk-info.bootIndex;
 +if (qemuDomainChangeBootIndex(driver, vm, orig_disk-info,
 +  disk-info.bootIndex ?
 +  disk-info.bootIndex : -1)  0)
 +goto end;
 +orig_disk-info.bootIndex = disk-info.bootIndex;
  }
  
  /* Add the new disk src into shared disk hash table */
 @@ -7075,6 +7085,16 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
  if (qemuDomainChangeEjectableMedia(driver, conn, vm,
 orig_disk, dev-data.disk-src, 
 force)  0) {
  ignore_value(qemuRemoveSharedDisk(driver, dev-data.disk, 
 vm-def-name));
 +
 +/* Change media failed. Boot index should be changed back to 
 original. */
 +if (oldBootIndex = 0) {
 +if (qemuDomainChangeBootIndex(driver, vm, orig_disk-info,
 +  oldBootIndex ? oldBootIndex : 
 -1)  0) {
 +VIR_WARN(Change Boot index back to original failed);
 +goto end;
 +}
 +orig_disk-info.bootIndex = oldBootIndex;
 +}
  goto end;
  }
  
 @@ -7082,6 +7102,22 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
  ret = 0;
  break;
  
 +case VIR_DOMAIN_DISK_DEVICE_DISK:
 +if (orig_disk-info.bootIndex != disk-info.bootIndex) {
 +if (qemuDomainChangeBootIndex(driver, vm, orig_disk-info,
 +  disk-info.bootIndex ?
 +  disk-info.bootIndex : -1)  0)
 +goto end;
 +orig_disk-info.bootIndex = disk-info.bootIndex;
 +ret = 0;
 +break;
 +} else {
 +

[libvirt] [PATCH v2 1/2] virsh-volume: add support for --reflink

2015-02-02 Thread Chen Hanxiao
add support for --reflink to specify
VIR_STORAGE_VOL_CREATE_REFLINK flag.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 tools/virsh-volume.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index d585ee2..db94154 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -204,6 +204,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
 
 if (vshCommandOptBool(cmd, prealloc-metadata))
 flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
+
 if (!(pool = vshCommandOptPool(ctl, cmd, pool, NULL)))
 return false;
 
@@ -378,6 +379,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
 
 if (vshCommandOptBool(cmd, prealloc-metadata))
 flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
+
 if (!(pool = vshCommandOptPool(ctl, cmd, pool, NULL)))
 return false;
 
@@ -441,6 +443,10 @@ static const vshCmdOptDef opts_vol_create_from[] = {
  .type = VSH_OT_BOOL,
  .help = N_(preallocate metadata (for qcow2 instead of full allocation))
 },
+{.name = reflink,
+ .type = VSH_OT_BOOL,
+ .help = N_(use btrfs COW lightweight copy)
+},
 {.name = NULL}
 };
 
@@ -460,6 +466,9 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptBool(cmd, prealloc-metadata))
 flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
 
+if (vshCommandOptBool(cmd, reflink))
+flags |= VIR_STORAGE_VOL_CREATE_REFLINK;
+
 if (vshCommandOptStringReq(ctl, cmd, file, from)  0)
 goto cleanup;
 
@@ -554,6 +563,10 @@ static const vshCmdOptDef opts_vol_clone[] = {
  .type = VSH_OT_BOOL,
  .help = N_(preallocate metadata (for qcow2 instead of full allocation))
 },
+{.name = reflink,
+ .type = VSH_OT_BOOL,
+ .help = N_(use btrfs COW lightweight copy)
+},
 {.name = NULL}
 };
 
@@ -574,6 +587,9 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptBool(cmd, prealloc-metadata))
 flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
 
+if (vshCommandOptBool(cmd, reflink))
+flags |= VIR_STORAGE_VOL_CREATE_REFLINK;
+
 origpool = virStoragePoolLookupByVolume(origvol);
 if (!origpool) {
 vshError(ctl, %s, _(failed to get parent pool));
-- 
2.1.0

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


Re: [libvirt] [PATCH] virsh-volume: add support for --reflink

2015-02-02 Thread Chen, Hanxiao


 -Original Message-
 From: John Ferlan [mailto:jfer...@redhat.com]
 Sent: Monday, February 02, 2015 7:52 PM
 To: Chen, Hanxiao/陈 晗霄; libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH] virsh-volume: add support for --reflink
 
 
 
 On 02/01/2015 10:14 PM, Chen Hanxiao wrote:
  add support for --reflink to specify
  VIR_STORAGE_VOL_CREATE_REFLINK flag.
 
  Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
  ---
   tools/virsh-volume.c | 16 
   1 file changed, 16 insertions(+)
 
 
 What about the man page (virsh.pod) to describe --reflink?
 

I'll add a man-page patch and resend it.

Thanks,
- Chen

 
  diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
  index d585ee2..db94154 100644
  --- a/tools/virsh-volume.c
  +++ b/tools/virsh-volume.c
  @@ -204,6 +204,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
 
   if (vshCommandOptBool(cmd, prealloc-metadata))
   flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
  +
   if (!(pool = vshCommandOptPool(ctl, cmd, pool, NULL)))
   return false;
 
  @@ -378,6 +379,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
 
   if (vshCommandOptBool(cmd, prealloc-metadata))
   flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
  +
   if (!(pool = vshCommandOptPool(ctl, cmd, pool, NULL)))
   return false;
 
  @@ -441,6 +443,10 @@ static const vshCmdOptDef opts_vol_create_from[] = {
.type = VSH_OT_BOOL,
.help = N_(preallocate metadata (for qcow2 instead of full 
  allocation))
   },
  +{.name = reflink,
  + .type = VSH_OT_BOOL,
  + .help = N_(use btrfs COW lightweight copy)
  +},
   {.name = NULL}
   };
 
  @@ -460,6 +466,9 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
   if (vshCommandOptBool(cmd, prealloc-metadata))
   flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
 
  +if (vshCommandOptBool(cmd, reflink))
  +flags |= VIR_STORAGE_VOL_CREATE_REFLINK;
  +
   if (vshCommandOptStringReq(ctl, cmd, file, from)  0)
   goto cleanup;
 
  @@ -554,6 +563,10 @@ static const vshCmdOptDef opts_vol_clone[] = {
.type = VSH_OT_BOOL,
.help = N_(preallocate metadata (for qcow2 instead of full 
  allocation))
   },
  +{.name = reflink,
  + .type = VSH_OT_BOOL,
  + .help = N_(use btrfs COW lightweight copy)
  +},
   {.name = NULL}
   };
 
  @@ -574,6 +587,9 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
   if (vshCommandOptBool(cmd, prealloc-metadata))
   flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
 
  +if (vshCommandOptBool(cmd, reflink))
  +flags |= VIR_STORAGE_VOL_CREATE_REFLINK;
  +
   origpool = virStoragePoolLookupByVolume(origvol);
   if (!origpool) {
   vshError(ctl, %s, _(failed to get parent pool));
 

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

Re: [libvirt] [PATCH 3/8] conf: check boot order which is used by itself

2015-02-02 Thread John Ferlan


On 01/05/2015 02:29 AM, Wang Rui wrote:
 We can update device and attach device(cdrom) with the boot order unchanged.
 But the boot order check method will report an error:
 boot order %d is already used by another device
 
 In fact the boot order is used by itself, we just want to keep it as before.
 This patch improves boot order check to distinguish the owner.
 
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  src/conf/device_conf.c | 13 +
  src/conf/device_conf.h | 12 
  src/conf/domain_conf.c | 31 +++
  src/conf/domain_conf.h |  9 -
  4 files changed, 52 insertions(+), 13 deletions(-)
 

Probably could have split this in two in order to do code motion fi...

3a. - Move virDomainDeviceDriveAddress from domain_conf.h to
device_conf.h which just seems to be separate from the... op

3b. - Rest of this patch and you will also need to modify
src/libvirt_private.syms in order to add virDeviceDriveAddressEqual;
properly since it's global.

I also think that there's some synergies with patch 6/8 especially with
the matching logic

Perhaps an update to the boot order text in the hostdev section of
formatdomain.html.in would be beneficial to describe these additional
checks.

 diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
 index 5ffe159..907a7b8 100644
 --- a/src/conf/device_conf.c
 +++ b/src/conf/device_conf.c
 @@ -144,6 +144,19 @@ virDevicePCIAddressEqual(virDevicePCIAddress *addr1,
  return false;
  }
  
 +bool
 +virDeviceDriveAddressEqual(virDomainDeviceDriveAddressPtr addr1,
 +   virDomainDeviceDriveAddressPtr addr2)
 +{
 +if (addr1-controller == addr2-controller 
 +addr1-bus == addr2-bus 
 +addr1-target == addr2-target 
 +addr1-unit == addr2-unit) {
 +return true;
 +}
 +return false;
 +}
 +
  int
  virInterfaceLinkParseXML(xmlNodePtr node,
   virInterfaceLinkPtr lnk)
 diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
 index f067a35..afcf0ee 100644
 --- a/src/conf/device_conf.h
 +++ b/src/conf/device_conf.h
 @@ -55,6 +55,15 @@ struct _virDevicePCIAddress {
  int  multi;  /* enum virDomainDeviceAddressPCIMulti */
  };
  
 +typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress;
 +typedef virDomainDeviceDriveAddress *virDomainDeviceDriveAddressPtr;
 +struct _virDomainDeviceDriveAddress {
 +unsigned int controller;
 +unsigned int bus;
 +unsigned int target;
 +unsigned int unit;
 +};
 +
  typedef struct _virInterfaceLink virInterfaceLink;
  typedef virInterfaceLink *virInterfaceLinkPtr;
  struct _virInterfaceLink {
 @@ -74,6 +83,9 @@ int virDevicePCIAddressFormat(virBufferPtr buf,
  bool virDevicePCIAddressEqual(virDevicePCIAddress *addr1,
virDevicePCIAddress *addr2);
  
 +bool virDeviceDriveAddressEqual(virDomainDeviceDriveAddressPtr addr1,
 +virDomainDeviceDriveAddressPtr addr2);
 +
  int virInterfaceLinkParseXML(xmlNodePtr node,
   virInterfaceLinkPtr lnk);
  
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index aafc05e..d2c4a0a 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -19953,12 +19953,35 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr 
 def ATTRIBUTE_UNUSED,
  virDomainDeviceInfoPtr newinfo = opaque;
  
  if (info-bootIndex == newinfo-bootIndex) {
 -virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 -   _(boot order %d is already used by another device),
 -   newinfo-bootIndex);
 -return -1;
 +/* check if the same boot order is used by another device or itself 
 */
 +if (info-type != newinfo-type) {
 +goto error;
 +} else {
 +switch ((virDomainDeviceAddressType) newinfo-type) {
 +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
 +if (virDevicePCIAddressEqual(info-addr.pci,
 + newinfo-addr.pci))
 +return 0;
 +else
 +goto error;
 +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
 +if (virDeviceDriveAddressEqual(info-addr.drive,
 +   newinfo-addr.drive))
 +return 0;
 +else
 +goto error;
 +default:
 +goto error;

This is difficult to read with all the if-then-else and goto's.

How about the following:

if (info-bootIndex == newinfo-bootIndex) {
/* check if the same boot order is used by another device or
itself */
if (info-type != newinfo-type)
goto error;

if ((newinfo-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI 
 virDevicePCIAddressEqual(info-addr.pci,

Re: [libvirt] [PATCH 5/8] qemu: support attachment of disk device with boot index

2015-02-02 Thread John Ferlan


On 01/05/2015 02:29 AM, Wang Rui wrote:
 When we attach a disk to a running VM with boot index, we can get a
 successful result. But in fact the boot index won't take effect. QEMU
 supported to set device's boot index online recently(since QEMU 2.2.0).
 
 After this patch, the boot index will take effect after
 virDomainAttachDevice(Flags) API returning success. If new disk is
 attached successfully but boot index is set failed, we'll remove the
 new disk to restore.
 
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  src/qemu/qemu_hotplug.c | 88 
 +
  1 file changed, 88 insertions(+)
 

If I read the tea leaves correctly - running this on  qemu 2.2 will
cause a failure since the ChangeBootIndex isn't supported there.

I don't get a sense why just failing the boot index setting should cause
the disk to not be attached.  How do you differentiate the failure is
not supported vs. not present?

Seems to me there should be a way to qom-get and do some pre-checks
before adding, then trying to rip it out which is just ripe for all
sorts of errors. Taking that route perhaps means the code movement patch
is unnecessary and the device removal is unnecessary.

I didn't dive deep into the following code as I really think the qom-get
is something that'll obsolete certain parts of it.

John
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 2f84949..5eacfce 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -2999,7 +2999,12 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
  {
  virDomainDiskDefPtr disk = dev-data.disk;
  virDomainDiskDefPtr orig_disk = NULL;
 +virDomainDeviceDefPtr new_dev_copy = NULL;
 +virDomainDeviceDefPtr old_dev_copy = NULL;
 +virDomainDiskDefPtr tmp = NULL;
 +virCapsPtr caps = NULL;
  int ret = -1;
 +int removed = 0;
  const char *driverName = virDomainDiskGetDriver(disk);
  const char *src = virDomainDiskGetSource(disk);
  
 @@ -3022,6 +3027,13 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
  if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true)  0)
  goto end;
  
 +if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 +goto end;
 +
 +if (!(new_dev_copy = virDomainDeviceDefCopy(dev, vm-def,
 +caps, driver-xmlopt)))
 +goto end;
 +
  switch (disk-device)  {
  case VIR_DOMAIN_DISK_DEVICE_CDROM:
  case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
 @@ -3036,12 +3048,33 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
  goto end;
  }
  
 +tmp = dev-data.disk;
 +dev-data.disk = orig_disk;
 +/* save a copy of old device to restore */
 +if (!(old_dev_copy = virDomainDeviceDefCopy(dev, vm-def,
 +caps, driver-xmlopt))) {
 +dev-data.disk = tmp;
 +goto end;
 +}
 +dev-data.disk = tmp;
 +
  if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
 disk-src, false)  0)
  goto end;
  
  disk-src = NULL;
  ret = 0;
 +
 +tmp = new_dev_copy-data.disk;
 +if (orig_disk-info.bootIndex != tmp-info.bootIndex) {
 +/* If boot index is to be changed to 0, we can pass value:-1 to
 +   qmp command(qom-set) to cancel boot index. */
 +if (qemuDomainChangeBootIndex(driver, vm, orig_disk-info,
 +  tmp-info.bootIndex ?
 +  tmp-info.bootIndex : -1)  0)
 +goto try_remove;
 +orig_disk-info.bootIndex = tmp-info.bootIndex;
 +}
  break;
  
  case VIR_DOMAIN_DISK_DEVICE_DISK:
 @@ -3063,6 +3096,15 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
 _(disk bus '%s' cannot be hotplugged.),
 virDomainDiskBusTypeToString(disk-bus));
  }
 +
 +tmp = new_dev_copy-data.disk;
 +if (!ret  tmp-info.bootIndex  0) {
 +if (qemuDomainChangeBootIndex(driver, vm, disk-info,
 +  tmp-info.bootIndex)  0)
 +goto try_remove;
 +disk-info.bootIndex = tmp-info.bootIndex;
 +}
 +
  break;
  default:
  virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
 @@ -3074,7 +3116,53 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
   end:
  if (ret != 0)
  ignore_value(qemuRemoveSharedDevice(driver, dev, vm-def-name));
 +virObjectUnref(caps);
 +virDomainDeviceDefFree(new_dev_copy);
 +virDomainDeviceDefFree(old_dev_copy);
 +if (removed) {
 +dev-data.disk = NULL;
 +ret = -1;
 +}
  return ret;
 +
 + try_remove:
 +if 

[libvirt] [PATCH 0/3] Prevent removing a in-used static bridge and destroying a in-used virtual network

2015-02-02 Thread Lin Ma
* Get the live state info of a virtual network through netcf in 
networkGetXMLDesc.
* Add --system flag for net-dumpxml to show the live state info.
* Check the live state info in net-destroy.
* Add --force flag for net-destroy to forcibly destroy the virtual network.
* Check the transient interfaces info in iface-unbridge.

---
Lin Ma (3):
  bridge_driver: Return the live state info of a given virtual network
  virsh: prevent destroying a in-used network for net-destroy
  virsh: prevent removing a in-used bridge for iface-unbridge

 include/libvirt/libvirt-network.h|   1 +
 src/Makefile.am  |   3 +
 src/network/bridge_driver.c  | 141 ++-
 src/network/bridge_driver_platform.h |   7 ++
 tests/Makefile.am|   4 +
 tools/virsh-interface.c  |  25 ++-
 tools/virsh-network.c|  62 ++-
 tools/virsh.pod  |   8 +-
 8 files changed, 241 insertions(+), 10 deletions(-)

-- 
1.8.4

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


[libvirt] [PATCH 2/3] virsh: prevent destroying a in-used network for net-destroy

2015-02-02 Thread Lin Ma
* add --system flag for net-dumpxml to show information about the
  attached interfaces of the virtual network.

* call virNetworkGetXMLDesc in cmdNetworkDestroy to get the live state
  info to check whether the virtual network is in use.

* add --force flag for net-destroy to forcibly destroy the virtual
  network even if it's in use.

Signed-off-by: Lin Ma l...@suse.com
---
 tools/virsh-network.c | 62 +++
 tools/virsh.pod   |  8 +--
 2 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 5f8743c..17a5710 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -253,6 +253,10 @@ static const vshCmdOptDef opts_network_destroy[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_(network name or uuid)
 },
+{.name = force,
+ .type = VSH_OT_BOOL,
+ .help = N_(Forcefully stop a given network)
+},
 {.name = NULL}
 };
 
@@ -260,20 +264,55 @@ static bool
 cmdNetworkDestroy(vshControl *ctl, const vshCmd *cmd)
 {
 virNetworkPtr network;
-bool ret = true;
+bool ret = false;
 const char *name;
+char *br_xml = NULL;
+xmlDocPtr xml_doc = NULL;
+xmlXPathContextPtr ctxt = NULL;
 
 if (!(network = vshCommandOptNetwork(ctl, cmd, name)))
-return false;
+goto cleanup;
+
+#ifdef WITH_NETCF
+if (!vshCommandOptBool(cmd, force)) {
+if (virNetworkIsActive(network) = 0) {
+vshError(ctl, %s, _(network is not active));
+goto cleanup;
+}
+if (!(br_xml = virNetworkGetXMLDesc(network,
+VIR_NETWORK_XML_IFACE_ATTACHED)))
+goto cleanup;
+
+if (!(xml_doc = virXMLParseStringCtxt(br_xml,
+  _((bridge interface 
+definition)), ctxt))) {
+vshError(ctl, %s, _(Failed to parse network configuration));
+goto cleanup;
+}
+
+if (virXPathNode(./bridge/interface[1], ctxt) != NULL) {
+vshError(ctl, %s, _(The network is in use by guests));
+vshPrint(ctl, %s, _(Use --force flag if you do want to do so));
+goto cleanup;
+}
+}
+#endif
 
 if (virNetworkDestroy(network) == 0) {
 vshPrint(ctl, _(Network %s destroyed\n), name);
 } else {
 vshError(ctl, _(Failed to destroy network %s), name);
-ret = false;
+goto cleanup;
 }
 
-virNetworkFree(network);
+ret = true;
+ cleanup:
+if(network)
+virNetworkFree(network);
+VIR_FREE(br_xml);
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml_doc);
+
 return ret;
 }
 
@@ -300,6 +339,10 @@ static const vshCmdOptDef opts_network_dumpxml[] = {
  .type = VSH_OT_BOOL,
  .help = N_(network information of an inactive domain)
 },
+{.name = system,
+ .type = VSH_OT_BOOL,
+ .help = N_(current live state information including attached interfaces)
+},
 {.name = NULL}
 };
 
@@ -311,6 +354,7 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
 char *dump;
 unsigned int flags = 0;
 int inactive;
+bool system = false;
 
 if (!(network = vshCommandOptNetwork(ctl, cmd, NULL)))
 return false;
@@ -319,6 +363,16 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
 if (inactive)
 flags |= VIR_NETWORK_XML_INACTIVE;
 
+system = vshCommandOptBool(cmd, system);
+if (system) {
+flags = VIR_NETWORK_XML_IFACE_ATTACHED;
+if (virNetworkIsActive(network) = 0) {
+vshError(ctl, %s, _(--system only works on active network));
+virNetworkFree(network);
+return false;
+}
+}
+
 dump = virNetworkGetXMLDesc(network, flags);
 
 if (dump != NULL) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 804458e..3373ada 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2672,16 +2672,20 @@ to get a description of the XML network format used by 
libvirt.
 Define a persistent virtual network from an XML Ifile, the network is just
 defined but not instantiated (started).
 
-=item Bnet-destroy Inetwork
+=item Bnet-destroy Inetwork  [I--force]
 
 Destroy (stop) a given transient or persistent virtual network
 specified by its name or UUID. This takes effect immediately.
+If I--force is specified, then forcefully destroy the virtual
+network even if it's in use.
 
-=item Bnet-dumpxml Inetwork [I--inactive]
+=item Bnet-dumpxml Inetwork [I--inactive] [I--system]
 
 Output the virtual network information as an XML dump to stdout.
 If I--inactive is specified, then physical functions are not
 expanded into their associated virtual functions.
+If I--system is specified, then directly output the current
+live state corresponding to this network from system.
 
 =item Bnet-edit Inetwork
 
-- 
1.8.4

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

[libvirt] [PATCH 1/3] bridge_driver: Return the live state info of a given virtual network

2015-02-02 Thread Lin Ma
It constructs a temporary static config of the network, Obtains all of
attached interfaces information through netcf, Then removes the config.

Signed-off-by: Lin Ma l...@suse.com
---
 include/libvirt/libvirt-network.h|   1 +
 src/Makefile.am  |   3 +
 src/network/bridge_driver.c  | 141 ++-
 src/network/bridge_driver_platform.h |   7 ++
 tests/Makefile.am|   4 +
 5 files changed, 155 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-network.h 
b/include/libvirt/libvirt-network.h
index 308f27f..9b09546 100644
--- a/include/libvirt/libvirt-network.h
+++ b/include/libvirt/libvirt-network.h
@@ -30,6 +30,7 @@
 
 typedef enum {
 VIR_NETWORK_XML_INACTIVE = (1  0), /* dump inactive network information 
*/
+VIR_NETWORK_XML_IFACE_ATTACHED = (1  1), /* dump current live state */
 } virNetworkXMLFlags;
 
 /**
diff --git a/src/Makefile.am b/src/Makefile.am
index b41c6d4..d22ae7e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1412,6 +1412,9 @@ if WITH_NETWORK
 noinst_LTLIBRARIES += libvirt_driver_network_impl.la
 libvirt_driver_network_la_SOURCES =
 libvirt_driver_network_la_LIBADD = libvirt_driver_network_impl.la
+if WITH_NETCF
+libvirt_driver_network_la_LIBADD += $(NETCF_LIBS)
+endif WITH_NETCF
 if WITH_DRIVER_MODULES
 mod_LTLIBRARIES += libvirt_driver_network.la
 libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la \
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c56e8f2..1e49e2e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -,8 +,17 @@ static char *networkGetXMLDesc(virNetworkPtr net,
 virNetworkObjPtr network;
 virNetworkDefPtr def;
 char *ret = NULL;
+#ifdef WITH_NETCF
+struct netcf_if *iface = NULL;
+char *bridge = NULL;
+char *if_xml_tmp = NULL;
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
+xmlXPathObjectPtr obj = NULL;
+#endif
 
-virCheckFlags(VIR_NETWORK_XML_INACTIVE, NULL);
+virCheckFlags(VIR_NETWORK_XML_INACTIVE |
+  VIR_NETWORK_XML_IFACE_ATTACHED, NULL);
 
 if (!(network = networkObjFromNetwork(net)))
 return ret;
@@ -3342,6 +3351,135 @@ static char *networkGetXMLDesc(virNetworkPtr net,
 if (virNetworkGetXMLDescEnsureACL(net-conn, network-def)  0)
 goto cleanup;
 
+#ifdef WITH_NETCF
+if ((flags  VIR_NETWORK_XML_INACTIVE)  network-newDef) {
+def = network-newDef;
+ret = virNetworkDefFormat(def, flags);
+}
+else if (flags  VIR_NETWORK_XML_IFACE_ATTACHED) {
+if (!(network-def-bridge)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(network '%s' does not have a bridge name.),
+   network-def-name);
+goto cleanup;
+}
+ignore_value(VIR_STRDUP(bridge, network-def-bridge));
+
+if (virAsprintf(if_xml_tmp,
+interface type='bridge' name='%s'
+start mode='none'/bridge//interface,
+bridge)  0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(failed to generate temp xml for network));
+goto cleanup;
+}
+
+if (ncf_init(driver-netcf, NULL) != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(failed to init netcf));
+   goto cleanup;
+}
+
+// create a temp bridge configuration file
+iface = ncf_define(driver-netcf, if_xml_tmp);
+if (!iface) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(failed to define the temp bridge %s), bridge);
+ncf_close(driver-netcf);
+goto cleanup;
+}
+
+ret = ncf_if_xml_state(iface);
+if (!ret) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(could not get bridge XML description));
+ncf_if_free(iface);
+ncf_close(driver-netcf);
+goto cleanup;
+}
+
+// remove the temp bridge configuration file
+if (ncf_if_undefine(iface)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(failed to undefine the temp bridge %s), bridge);
+ncf_if_free(iface);
+ncf_close(driver-netcf);
+ret = NULL;
+goto cleanup;
+}
+ncf_if_free(iface);
+ncf_close(driver-netcf);
+
+// remove the dummp tap interface section from the result
+if (network-def-mac_specified) {
+char *macTapIfName = 
networkBridgeDummyNicName(network-def-bridge);
+if (macTapIfName) {
+char mac[VIR_MAC_STRING_BUFLEN];
+xmlNodePtr cur = NULL, matchNode = NULL;
+xmlChar *br_xml = NULL;
+int br_xml_size;
+char buf[64];
+   

[libvirt] [PATCH 3/3] virsh: prevent removing a in-used bridge for iface-unbridge

2015-02-02 Thread Lin Ma
By checking transient interfaces, It obtains the live information of
attached interfaces to see if the bridge is in use.

Signed-off-by: Lin Ma l...@suse.com
---
 tools/virsh-interface.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index 5f848b6..ff40be0 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -1040,11 +1040,11 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
 const char *br_name;
 char *if_type = NULL, *if_name = NULL;
 bool nostart = false;
-char *br_xml = NULL;
+char *br_xml = NULL, *br_xml_transient_if = NULL;
 xmlChar *if_xml = NULL;
 int if_xml_size;
-xmlDocPtr xml_doc = NULL;
-xmlXPathContextPtr ctxt = NULL;
+xmlDocPtr xml_doc = NULL, xml_doc_transient_if = NULL;
+xmlXPathContextPtr ctxt = NULL, ctxt_transient_if = NULL;
 xmlNodePtr top_node, if_node, cur;
 
 /* Get a handle to the original device */
@@ -1103,6 +1103,22 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
 goto cleanup;
 }
 
+/* verify whether there is any transient interface attached to bridge. */
+if (!(br_xml_transient_if = virInterfaceGetXMLDesc(br_handle, 0)))
+goto cleanup;
+
+if (!(xml_doc_transient_if = virXMLParseStringCtxt(br_xml_transient_if,
+   _((bridge interface 
definition)),
+   ctxt_transient_if))) {
+vshError(ctl, _(Failed to parse configuration of %s), br_name);
+goto cleanup;
+}
+
+if (virXPathNode(./bridge/interface[2], ctxt_transient_if) != NULL) {
+vshError(ctl, %s, _(The bridge is in use by transient interfaces));
+goto cleanup;
+}
+
 /* Change the type and name of the outer/master interface to
  * the type/name of the attached slave interface.
  */
@@ -1198,10 +1214,13 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
virInterfaceFree(br_handle);
 VIR_FREE(if_xml);
 VIR_FREE(br_xml);
+VIR_FREE(br_xml_transient_if);
 VIR_FREE(if_type);
 VIR_FREE(if_name);
 xmlXPathFreeContext(ctxt);
+xmlXPathFreeContext(ctxt_transient_if);
 xmlFreeDoc(xml_doc);
+xmlFreeDoc(xml_doc_transient_if);
 return ret;
 }
 
-- 
1.8.4

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


Re: [libvirt] [PATCH] storage: Need to clear pool prior to refreshPool during Autostart

2015-02-02 Thread John Ferlan


On 02/02/2015 03:31 AM, Ján Tomko wrote:
 On Fri, Jan 30, 2015 at 02:25:18PM -0500, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1176510

 When storageDriverAutostart is called path virStateReload via a 'service
 libvirtd reload', then because the volume list in the pool wasn't cleared
 prior to the call, each volume would be listed multiple times (as many
 times as we reload). I believe the issue would be introduced by commit
 id '9e093f0b' at least for the libvirtd reload path, although I suppose
 the introduction of virStateReload (commit id '70da0494') could be a
 different cause.

 Thus like other places prior to calling refreshPool, we need to call
 virStoragePoolObjClearVols

 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/storage/storage_driver.c | 1 +
  1 file changed, 1 insertion(+)

 
 ACK, also worthy of backporting to v1.2.9-maint.
 
 Jan
 

OK - did that too.

Tks,

John

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


[libvirt] [PATCH v2] Make tests independant of system page size

2015-02-02 Thread Daniel P. Berrange
Some code paths have special logic depending on the page size
reported by sysconf, which in turn affects the test results.
We must mock this so tests always have a consistent page size.
---
 cfg.mk   |  8 
 src/libvirt_private.syms |  2 ++
 src/openvz/openvz_util.c |  5 +
 src/qemu/qemu_command.c  |  6 +++---
 src/qemu/qemu_driver.c   |  6 +-
 src/util/virfile.c   |  2 +-
 src/util/virnuma.c   |  6 +++---
 src/util/virutil.c   | 13 +
 src/util/virutil.h   |  3 +++
 src/xen/xen_hypervisor.c |  4 ++--
 tests/qemuxml2argvmock.c |  8 
 tests/qemuxml2argvtest.c |  1 +
 12 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 70612f8..d72b039 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -997,6 +997,11 @@ sc_prohibit_virXXXFree:
halt='avoid using 'virXXXFree', use 'virObjectUnref' instead' \
  $(_sc_search_regexp)
 
+sc_prohibit_sysconf_pagesize:
+   @prohibit='sysconf\(_SC_PAGESIZE' \
+   halt='use virGetSystemPageSize[KB] instead of sysconf(_SC_PAGESIZE)' \
+ $(_sc_search_regexp)
+
 # We don't use this feature of maint.mk.
 prev_version_file = /dev/null
 
@@ -1187,3 +1192,6 @@ exclude_file_name_regexp--sc_prohibit_devname = \
 
 exclude_file_name_regexp--sc_prohibit_virXXXFree = \
   
^(docs/|tests/|examples/|tools/|cfg.mk|src/test/test_driver.c|src/libvirt_public.syms|include/libvirt/libvirt-(domain|network|nodedev|storage|stream|secret|nwfilter|interface|domain-snapshot).h|src/libvirt-(domain|qemu|network|nodedev|storage|stream|secret|nwfilter|interface|domain-snapshot).c$$)
+
+exclude_file_name_regexp--sc_prohibit_sysconf_pagesize = \
+  ^(cfg\.mk|src/util/virutil\.c)$$
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f7f8ea2..376c69b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2228,6 +2228,8 @@ virGetListenFDs;
 virGetSCSIHostNameByParentaddr;
 virGetSCSIHostNumber;
 virGetSelfLastChanged;
+virGetSystemPageSize;
+virGetSystemPageSizeKB;
 virGetUnprivSGIOSysfsPath;
 virGetUserCacheDirectory;
 virGetUserConfigDirectory;
diff --git a/src/openvz/openvz_util.c b/src/openvz/openvz_util.c
index 8032f6a..3cdc1c2 100644
--- a/src/openvz/openvz_util.c
+++ b/src/openvz/openvz_util.c
@@ -42,10 +42,7 @@ openvzKBPerPages(void)
 static long kb_per_pages;
 
 if (kb_per_pages == 0) {
-kb_per_pages = sysconf(_SC_PAGESIZE);
-if (kb_per_pages  0) {
-kb_per_pages /= 1024;
-} else {
+if ((kb_per_pages = virGetSystemPageSizeKB())  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Can't determine page size));
 kb_per_pages = 0;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ec4f35b..3b6eddc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4540,7 +4540,7 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 virDomainHugePagePtr master_hugepage = NULL;
 virDomainHugePagePtr hugepage = NULL;
 virDomainNumatuneMemMode mode;
-const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+const long system_page_size = virGetSystemPageSizeKB();
 virMemAccess memAccess = def-cpu-cells[guestNode].memAccess;
 size_t i;
 char *mem_path = NULL;
@@ -7051,7 +7051,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 bool needBackend = false;
 int rc;
 int ret = -1;
-const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+const long system_page_size = virGetSystemPageSizeKB();
 
 if (virDomainNumatuneHasPerNodeBinding(def-numatune) 
 !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
@@ -8239,7 +8239,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 def-mem.max_balloon = VIR_DIV_UP(def-mem.max_balloon, 1024) * 1024;
 virCommandAddArgFormat(cmd, %llu, def-mem.max_balloon / 1024);
 if (def-mem.nhugepages  (!def-cpu || !def-cpu-ncells)) {
-const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+const long system_page_size = virGetSystemPageSizeKB();
 char *mem_path = NULL;
 
 if (def-mem.hugepages[0].size == system_page_size) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 59a9593..cf351e6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1352,12 +1352,8 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int 
*lastCpu, long *vm_rss,
 if (lastCpu)
 *lastCpu = cpu;
 
-/* We got pages
- * We want kiloBytes
- * _SC_PAGESIZE is page size in Bytes
- * So calculate, but first lower the pagesize so we don't get overflow */
 if (vm_rss)
-*vm_rss = rss * (sysconf(_SC_PAGESIZE)  10);
+*vm_rss = rss * virGetSystemPageSizeKB();
 
 
 VIR_DEBUG(Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld,
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 4024f3d..68ef6eb 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ 

Re: [libvirt] [PATCH] Make tests independant of system page size

2015-02-02 Thread Eric Blake
On 02/02/2015 05:32 AM, Michal Privoznik wrote:
 On 02.02.2015 12:10, Daniel P. Berrange wrote:
 Some code paths have special logic depending on the page size
 reported by sysconf, which in turn affects the test results.
 We must mock this so tests always have a consistent page size.
 ---

 
 Should we also introduce a syntax-check rule to prefer
 virGetSystemPageSize*()?

Yes. Need a hand writing it?

-- 
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] Missing libxl_device_nic settings

2015-02-02 Thread Cedric Bosdonnat
Hi Jihoon,

On Sat, 2015-01-24 at 17:17 +, Kim Larry wrote:
 I was trying to pass ip address to scripts/vif-bridge by putting ip
 address=/ in guest config xml file, however, I found that
 libxlMakeNic(which located in libxl/libxl_conf.c:956) doesn't set
 x_nic-ip. So I patched myself but I'm not so sure
 about VIR_DOMAIN_NET_TYPE_ETHERNET. It seems like vif-route, correct? 

You're right that the libxl driver needs to support that feature.
However you should pull lastest sources from master branch as I pushed
pretty big changes in the network configuration recently.

 Here is my patch:
 
 
 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
 index 0555b91..0effc59 100644
 --- a/src/libxl/libxl_conf.c
 +++ b/src/libxl/libxl_conf.c
 @@ -1047,10 +1047,18 @@ libxlMakeNic(virDomainDefPtr def,
  if (VIR_STRDUP(x_nic-bridge,
 virDomainNetGetActualBridgeName(l_nic)) 
 0)
  return -1;
 -/* fallthrough */
 +if (VIR_STRDUP(x_nic-script, l_nic-script)  0)
 +return -1;
 +if (VIR_STRDUP(x_nic-ip, l_nic-data.bridge.ipaddr)  0)
 +return -1;
 +   break;

Now, domains can have multiple IPs: l_nic-data.bridge.ipaddr doesn't
exist anymore, you'll need to read from l_nic-ips (array of size
l_nic-nips).

I hope that helps,

--
Cedric


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


Re: [libvirt] [PATCH 6/8] conf: add compatiblity check for boot index when updating device

2015-02-02 Thread John Ferlan


On 01/05/2015 02:29 AM, Wang Rui wrote:
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  src/conf/domain_conf.c | 43 +++
  1 file changed, 23 insertions(+), 20 deletions(-)
 

Since there's no commit message - even more reason to combine with patch
3 (or my 3b)


 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index d2c4a0a..5beb830 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -19991,29 +19991,32 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
  {
  virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
  
 -if (action != VIR_DOMAIN_DEVICE_ACTION_ATTACH)
 -return 0;
 -
 -if (!virDomainDefHasUSB(def) 
 -STRNEQ(def-os.type, exe) 
 -virDomainDeviceIsUSB(dev)) {
 -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 -   _(Device configuration is not compatible: 
 - Domain has no USB bus support));
 -return -1;
 -}
 -
 -if (info  info-bootIndex  0) {
 -if (def-os.nBootDevs  0) {
 +switch (action) {
 +case VIR_DOMAIN_DEVICE_ACTION_ATTACH:
 +if (!virDomainDefHasUSB(def) 
 +STRNEQ(def-os.type, exe) 
 +virDomainDeviceIsUSB(dev)) {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 -   _(per-device boot elements cannot be used
 -  together with os/boot elements));
 +   _(Device configuration is not compatible: 
 + Domain has no USB bus support));
  return -1;
  }
 -if (virDomainDeviceInfoIterate(def,
 -   virDomainDeviceInfoCheckBootIndex,
 -   info)  0)
 -return -1;
 +case VIR_DOMAIN_DEVICE_ACTION_UPDATE:
 +if (info  info-bootIndex  0) {
 +if (def-os.nBootDevs  0) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(per-device boot elements cannot be used
 +  together with os/boot elements));
 +return -1;
 +}
 +if (virDomainDeviceInfoIterate(def,
 +   virDomainDeviceInfoCheckBootIndex,
 +   info)  0)
 +return -1;
 +}
 +break;
 +default:
 +return 0;

The 'norm' of using default is to list all the cases - makes it easier
to determine when one is missing.  I think the switch in this case is
overkill.


I think this is better:

if (action != VIR_DOMAIN_DEVICE_ACTION_ATTACH ||
action != VIR_DOMAIN_DEVICE_ACTION_UPDATE)
return 0;

if (action == VIR_DOMAIN_DEVICE_ACTION_ATTACH 
!virDomainDefHasUSB(def) 
STRNEQ(def-os.type, exe) 
virDomainDeviceIsUSB(dev)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
   _(Device configuration is not compatible: 
 Domain has no USB bus support));
return -1;
}

 if (info  info-bootIndex  0) {
 if (def-os.nBootDevs  0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(per-device boot elements cannot be used
   together with os/boot elements));
 return -1;
 }
 if (virDomainDeviceInfoIterate(def,
virDomainDeviceInfoCheckBootIndex,
info)  0)
 return -1;
 }


[sorry about the wrapping on the CheckBootIndex call - the point is all
you're doing is adding _UPDATE to the initial check... then adding the
if (action == *_ATTACH) for just the if USB check...


  }
  
  return 0;
 

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


Re: [libvirt] [PATCH 4/8] qemu: a code movement

2015-02-02 Thread John Ferlan


On 01/05/2015 02:29 AM, Wang Rui wrote:
 Move implementation of qemuDomainAttachDeviceDiskLive after
 qemuDomainDetachDiskDevice for later usage.
 
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 Signed-off-by: Zhou Yimin zhouyi...@huawei.com
 ---
  src/qemu/qemu_hotplug.c | 173 
 
  1 file changed, 86 insertions(+), 87 deletions(-)
 

Code motion - ACK.


John

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


[libvirt] [PATCH v2 2/2] man: add --reflink man page of virsh

2015-02-02 Thread Chen Hanxiao
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 tools/virsh.pod | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 804458e..c0bd99b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -3160,6 +3160,7 @@ BExample
 
 =item Bvol-create-from Ipool-or-uuid IFILE [I--inputpool
 Ipool-or-uuid] Ivol-name-or-key-or-path [I--prealloc-metadata]
+[I--reflink]
 
 Create a volume, using another volume as input.
 Ipool-or-uuid is the name or UUID of the storage pool to create the volume 
in.
@@ -3171,6 +3172,9 @@ Ivol-name-or-key-or-path is the name or key or path of 
the source volume.
 support full allocation). This option creates a sparse image file with 
metadata,
 resulting in higher performance compared to images with no preallocation and
 only slightly higher initial disk space usage.
+[I--reflink] When --reflink is specified, perform a COW lightweight copy,
+where the data blocks are copied only when modified.
+If this is not possible, the copy fails.
 
 =item Bvol-create-as Ipool-or-uuid Iname Icapacity
 [I--allocation Isize] [I--format Istring] [I--backing-vol
@@ -3205,7 +3209,7 @@ resulting in higher performance compared to images with 
no preallocation and
 only slightly higher initial disk space usage.
 
 =item Bvol-clone [I--pool Ipool-or-uuid] Ivol-name-or-key-or-path
-Iname [I--prealloc-metadata]
+Iname [I--prealloc-metadata] [I--reflink]
 
 Clone an existing volume.  Less powerful, but easier to type, version of
 Bvol-create-from.
@@ -3217,6 +3221,9 @@ Iname is the name of the new volume.
 support full allocation). This option creates a sparse image file with 
metadata,
 resulting in higher performance compared to images with no preallocation and
 only slightly higher initial disk space usage.
+[I--reflink] When --reflink is specified, perform a COW lightweight copy,
+where the data blocks are copied only when modified.
+If this is not possible, the copy fails.
 
 =item Bvol-delete [I--pool Ipool-or-uuid] Ivol-name-or-key-or-path
 
-- 
2.1.0

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


[libvirt] [PATCH v2 0/2] add support for --reflink to virsh

2015-02-02 Thread Chen Hanxiao
v2: add man page patch

Chen Hanxiao (2):
  virsh-volume: add support for --reflink
  man: add --reflink man page of virsh

 tools/virsh-volume.c | 16 
 tools/virsh.pod  |  9 -
 2 files changed, 24 insertions(+), 1 deletion(-)

-- 
2.1.0

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


[libvirt] [PATCH] virnetdev: fix some issues found by coverity and mingw builds

2015-02-02 Thread Pavel Hrdina
Commit e562a61a introduced new function to get/set interface state but
there was misuse of ATTRIBUTE_NONNULL on non-pointer attributes and also
we need to wrap that functions by #ifdef to not break mingw build.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 src/util/virnetdev.c | 94 ++--
 src/util/virnetdev.h | 12 +++
 2 files changed, 96 insertions(+), 10 deletions(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 7a0a43d..d8a4867 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -671,12 +671,23 @@ int virNetDevSetIFFlag(const char *ifname,
  *
  * Returns 0 in case of success or -1 on error.
  */
+#if defined(SIOCSIFFLAGS)  defined(HAVE_STRUCT_IFREQ)
 int virNetDevSetOnline(const char *ifname,
bool online)
 {
 
 return virNetDevSetIFFlag(ifname, IFF_UP, online);
 }
+#else
+int virNetDevSetOnline(const char *ifname,
+   bool online ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS,
+ _(Cannot set interface flags on '%s'),
+ ifname);
+return -1;
+}
+#endif
 
 /**
  * virNetDevSetPromiscuous:
@@ -689,11 +700,22 @@ int virNetDevSetOnline(const char *ifname,
  *
  * Returns 0 in case of success or -1 on error.
  */
+#if defined(SIOCSIFFLAGS)  defined(HAVE_STRUCT_IFREQ)
 int virNetDevSetPromiscuous(const char *ifname,
 bool promiscuous)
 {
 return virNetDevSetIFFlag(ifname, IFF_PROMISC, promiscuous);
 }
+#else
+int virNetDevSetPromiscuous(const char *ifname,
+bool promiscuous ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS,
+ _(Cannot set interface flags on '%s'),
+ ifname);
+return -1;
+}
+#endif
 
 /**
  * virNetDevSetRcvMulti:
@@ -707,11 +729,22 @@ int virNetDevSetPromiscuous(const char *ifname,
  *
  * Returns 0 in case of success or -1 on error.
  */
+#if defined(SIOCSIFFLAGS)  defined(HAVE_STRUCT_IFREQ)
 int virNetDevSetRcvMulti(const char *ifname,
  bool receive)
 {
 return virNetDevSetIFFlag(ifname, IFF_MULTICAST, receive);
 }
+#else
+int virNetDevSetRcvMulti(const char *ifname,
+ bool receive ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS,
+ _(Cannot set interface flags on '%s'),
+ ifname);
+return -1;
+}
+#endif
 
 /**
  * virNetDevSetRcvAllMulti:
@@ -723,11 +756,22 @@ int virNetDevSetRcvMulti(const char *ifname,
  *
  * Returns 0 in case of success or -1 on error.
  */
+#if defined(SIOCSIFFLAGS)  defined(HAVE_STRUCT_IFREQ)
 int virNetDevSetRcvAllMulti(const char *ifname,
 bool receive)
 {
 return virNetDevSetIFFlag(ifname, IFF_ALLMULTI, receive);
 }
+#else
+int virNetDevSetRcvAllMulti(const char *ifname,
+bool receive ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS,
+ _(Cannot set interface flags on '%s'),
+ ifname);
+return -1;
+}
+#endif
 
 
 #if defined(SIOCGIFFLAGS)  defined(HAVE_STRUCT_IFREQ)
@@ -776,11 +820,22 @@ int virNetDevGetIFFlag(const char *ifname,
  *
  * Returns 0 in case of success or an errno code in case of failure.
  */
+#if defined(SIOCSIFFLAGS)  defined(HAVE_STRUCT_IFREQ)
 int virNetDevGetOnline(const char *ifname,
bool *online)
 {
 return virNetDevGetIFFlag(ifname, IFF_UP, online);
 }
+#else
+int virNetDevGetOnline(const char *ifname,
+   bool *online ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS,
+ _(Cannot get interface flags on '%s'),
+ ifname);
+return -1;
+}
+#endif
 
 /**
  * virNetDevIsPromiscuous:
@@ -792,11 +847,22 @@ int virNetDevGetOnline(const char *ifname,
  *
  * Returns 0 in case of success or an errno code in case of failure.
  */
+#if defined(SIOCSIFFLAGS)  defined(HAVE_STRUCT_IFREQ)
 int virNetDevGetPromiscuous(const char *ifname,
-   bool *promiscuous)
+bool *promiscuous)
 {
 return virNetDevGetIFFlag(ifname, IFF_PROMISC, promiscuous);
 }
+#else
+int virNetDevGetPromiscuous(const char *ifname,
+bool *promiscuous ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS,
+ _(Cannot get interface flags on '%s'),
+ ifname);
+return -1;
+}
+#endif
 
 /**
  * virNetDevIsRcvMulti:
@@ -808,11 +874,22 @@ int virNetDevGetPromiscuous(const char *ifname,
  *
  * Returns 0 in case of success or -1 on error.
  */
+#if defined(SIOCSIFFLAGS)  defined(HAVE_STRUCT_IFREQ)
 int virNetDevGetRcvMulti(const char *ifname,
-bool *receive)
+ bool *receive)
 {
 return virNetDevGetIFFlag(ifname, IFF_MULTICAST, receive);
 }
+#else
+int virNetDevGetRcvMulti(const char 

[libvirt] [RFC] [Patch] Make hugepage testcase arch-agnostic

2015-02-02 Thread Prerna Saxena
Hi,
I have attached this patch as a response to a recent failure observed on 
PowerPC architecture by commit
311b4a67.
This patch introduces a check for dynamically obtaining system page size for 
test hugepages-pages6 under 'qemuxml2argv' suite. ( See patch for more verbose 
problem description)
This patch is not the most perfect implementation -- it fails syntax check; and 
has a Makefile-driven cleanup pending. I will be happy to deck it up and send 
it if the community concurs with this
approach.

We could also implement this via a shell script ( just like 
'virt-test-aa-helper')  but I couldnt find an easy way to determine host page 
size.

Awaiting community responses,
Prerna

From 8a64d4d22e2e65158d3caa45b615ca9a263f841f Mon Sep 17 00:00:00 2001
From: Prerna Saxena pre...@linux.vnet.ibm.com
Date: Mon, 2 Feb 2015 10:48:48 +0530
Subject: [PATCH] Commit 311b4a67 introduces a test for normal-page backed
 guest XML. However, it hardcodes the page size to 4 KB which is only valid
 for Intel Make check consequently fails on PowerPC where page size is 64KB

This makes the hugepages-pages6 test more modular, and enables the page size
to be picked up at runtime.
---
 .../qemuxml2argv-hugepages-pages6.template | 32 ++
 tests/qemuxml2argvtest.c   | 24 +++-
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.template

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.template 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.template
new file mode 100644
index 000..8b9b995
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.template
@@ -0,0 +1,32 @@
+domain type='qemu'
+  nameSomeDummyHugepagesGuest/name
+  uuidef1bdff4-27f3-4e85-a807-5fb4d58463cc/uuid
+  memory unit='KiB'1048576/memory
+  currentMemory unit='KiB'1048576/currentMemory
+  memoryBacking
+hugepages
+  page size='%llu' unit='KiB'/
+/hugepages
+  /memoryBacking
+  vcpu placement='static'2/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='ide' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='virtio'/
+  /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 89afa81..a16d937 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -479,6 +479,11 @@ mymain(void)
 {
 int ret = 0;
 bool skipLegacyCPUs = false;
+const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+int fd_in;
+FILE *f_out;
+char *template, *xml = NULL;
+char buf[1000];
 
 abs_top_srcdir = getenv(abs_top_srcdir);
 if (!abs_top_srcdir)
@@ -702,7 +707,24 @@ mymain(void)
 DO_TEST_FAILURE(hugepages-pages4, QEMU_CAPS_MEM_PATH,
 QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
 DO_TEST(hugepages-pages5, QEMU_CAPS_MEM_PATH);
-DO_TEST(hugepages-pages6, NONE);
+
+if (virAsprintf(template, %s/qemuxml2argvdata/qemuxml2argv-%s.template,
+abs_srcdir, hugepages-pages6)  0 ||
+virAsprintf(xml, %s/qemuxml2argvdata/qemuxml2argv-%s.xml,
+abs_srcdir, hugepages-pages6)  0)
+return EXIT_FAILURE;
+fd_in = open(template, O_RDONLY);
+f_out = fopen(xml, w);
+
+if ( fd_in != -1  f_out != NULL ) {
+if(read(fd_in, buf, sizeof(buf))) {
+fprintf(f_out, buf,system_page_size);
+fclose(f_out);
+close(fd_in);
+DO_TEST(hugepages-pages6, NONE);
+}
+}
+
 DO_TEST(nosharepages, QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MEM_MERGE);
 DO_TEST(disk-cdrom, NONE);
 DO_TEST(disk-cdrom-network-http, QEMU_CAPS_KVM, QEMU_CAPS_DEVICE,
-- 
1.9.3

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

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


Re: [libvirt] [PATCH] storage: Need to clear pool prior to refreshPool during Autostart

2015-02-02 Thread Ján Tomko
On Fri, Jan 30, 2015 at 02:25:18PM -0500, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1176510
 
 When storageDriverAutostart is called path virStateReload via a 'service
 libvirtd reload', then because the volume list in the pool wasn't cleared
 prior to the call, each volume would be listed multiple times (as many
 times as we reload). I believe the issue would be introduced by commit
 id '9e093f0b' at least for the libvirtd reload path, although I suppose
 the introduction of virStateReload (commit id '70da0494') could be a
 different cause.
 
 Thus like other places prior to calling refreshPool, we need to call
 virStoragePoolObjClearVols
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/storage/storage_driver.c | 1 +
  1 file changed, 1 insertion(+)
 

ACK, also worthy of backporting to v1.2.9-maint.

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] [Patch] Make hugepage testcase arch-agnostic

2015-02-02 Thread Ján Tomko
On Mon, Feb 02, 2015 at 01:16:31PM +0530, Prerna Saxena wrote:
 Hi,
 I have attached this patch as a response to a recent failure observed on 
 PowerPC architecture by commit
 311b4a67.
 This patch introduces a check for dynamically obtaining system page size for 
 test hugepages-pages6 under 'qemuxml2argv' suite. ( See patch for more 
 verbose problem description)
 This patch is not the most perfect implementation -- it fails syntax check; 
 and has a Makefile-driven cleanup pending. I will be happy to deck it up and 
 send it if the community concurs with this
 approach.
 
 We could also implement this via a shell script ( just like 
 'virt-test-aa-helper')  but I couldnt find an easy way to determine host page 
 size.
 
 Awaiting community responses,
 Prerna
 

My preferred solution would be separating all the sysconf(_SC_PAGESIZE)
calls into some virGetSystemPageSize helper and mocking it in
qemuxml2argvmock.c, always returning 4 KB.

Jan

 From 8a64d4d22e2e65158d3caa45b615ca9a263f841f Mon Sep 17 00:00:00 2001
 From: Prerna Saxena pre...@linux.vnet.ibm.com
 Date: Mon, 2 Feb 2015 10:48:48 +0530
 Subject: [PATCH] Commit 311b4a67 introduces a test for normal-page backed
  guest XML. However, it hardcodes the page size to 4 KB which is only valid
  for Intel Make check consequently fails on PowerPC where page size is 64KB
 
 This makes the hugepages-pages6 test more modular, and enables the page size
 to be picked up at runtime.
 ---
  .../qemuxml2argv-hugepages-pages6.template | 32 
 ++
  tests/qemuxml2argvtest.c   | 24 +++-
  2 files changed, 55 insertions(+), 1 deletion(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.template
 



signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list