Re: [libvirt] [PATCH v2 0/3] vz: implement some API calls

2015-10-20 Thread Dmitry Guryanov 

On 10/19/2015 10:02 PM, Maxim Nestratov wrote:

v1-v2 change:
   - removed vzConnectGetType
   - addressed comments on vzConnectGetMaxVcpus

Maxim Nestratov (3):
   vz: implement connectGetMaxVcpus API calls
   vz: implement API calls of nodeGetxxx family
   vz: implement some domain API calls

  src/vz/vz_driver.c | 109 +
  1 file changed, 109 insertions(+)


ACKed and pushed this series, thanks!

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

Re: [libvirt] [PATCH 2/2] Prefer STREQ and STRNEQ

2015-10-20 Thread Ishmanpreet Khera 
Ping?

On Wed, Oct 7, 2015 at 11:02 PM, Ishmanpreet Khera 
wrote:

> new syntax-check rule for !STREQ and !STRNEQ so that
> we don't end up in the same situation again.
>
> Signed-off-by: Ishmanpreet Kaur Khera 
> ---
>  cfg.mk | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/cfg.mk b/cfg.mk
> index e436434..7343dfc 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -1011,6 +1011,18 @@ sc_prohibit_pthread_create:
>  halt="avoid using 'pthread_create', use 'virThreadCreate' instead" \
>$(_sc_search_regexp)
>
> +sc_prohibit_not_streq:
> +   @prohibit='!STREQ'\
> +   exclude='exempt from syntax-check'\
> +   halt='Use STRNEQ instead of !STREQ'\
> + $(_sc_search_regexp)
> +
> +sc_prohibit_not_strneq:
> +   @prohibit='!STRNEQ'\
> +   exclude='exempt from syntax-check'\
> +   halt='Use STREQ instead of !STRNEQ'\
> + $(_sc_search_regexp)
> +
>  # We don't use this feature of maint.mk.
>  prev_version_file = /dev/null
>
> @@ -1213,3 +1225,9 @@
> exclude_file_name_regexp--sc_prohibit_sysconf_pagesize = \
>
>  exclude_file_name_regexp--sc_prohibit_pthread_create = \
>^(cfg\.mk|src/util/virthread\.c|tests/.*)$$
> +
> +exclude_file_name_regexp--sc_prohibit_not_streq = \
> +  ^tests/.*\.[ch]$$
> +
> +exclude_file_name_regexp--sc_prohibit_not_strneq = \
> +  ^tests/.*\.[ch]$$
> --
> 1.9.1
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] gem install ruby-libvirt fails on FreeBSD 10.2

2015-10-20 Thread Roman Bogorodskiy 
  Rickard von Essen wrote:

> Hi,
> 
> Installing the ruby-libvirt gem fails on FreeBSD 10.2 since it can't locate
> the lib and include dir. Installing with:
>   gem install ruby-libvirt --
> --with-libvirt-include=/usr/local/include/libvirt
> --with-libvirt-lib=/usr/local/lib/libvirt.so
> works fine.
> 
> It would be great if this worked out of the box, e.g. detects that it is
> building on BSD and applies the above settings as default, or at least it
> could be provided as a hint if running make fails to guide ruby noobs, like
> me.

Hi Rickard,

I tried it on my system and it seems to work fine without manually
providing include and lib paths.

>From what I can see, it uses pkg-config to detect CFLAGS and LDFLAGS (I
could be wrong here though because I'm not familiar with Ruby).

Could you please check if you have this working:

$ pkg-config --libs --cflags libvirt
-I/usr/local/include -L/usr/local/lib -lvirt
$

If that does not work, could you please describe how did you do the
libvirt installation on your system?

Thanks,

Roman Bogorodskiy

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

Re: [libvirt] [PATCH 1/2] Prefer STREQ and STRNEQ

2015-10-20 Thread Michal Privoznik 
On 07.10.2015 19:31, Ishmanpreet Khera wrote:
> use STRNEQ instead of !STREQ to remove inconsistency.
> 
> Signed-off-by: Ishmanpreet Kaur Khera 
> ---
>  src/bhyve/bhyve_driver.c  |  2 +-
>  src/conf/network_conf.c   |  4 ++--
>  src/conf/nwfilter_conf.c  |  2 +-
>  src/conf/nwfilter_params.c|  2 +-
>  src/conf/storage_conf.c   |  2 +-
>  src/lxc/lxc_fuse.c|  4 ++--
>  src/openvz/openvz_driver.c|  2 +-
>  src/qemu/qemu_command.c   |  6 +++---
>  src/qemu/qemu_domain.c|  2 +-
>  src/qemu/qemu_hotplug.c   |  2 +-
>  src/security/security_manager.c   |  2 +-
>  src/security/security_selinux.c   | 12 ++--
>  src/storage/storage_backend_logical.c |  2 +-
>  src/util/virfile.c|  2 +-
>  src/util/virsystemd.c |  2 +-
>  src/vz/vz_driver.c|  2 +-
>  src/vz/vz_sdk.c   |  2 +-
>  src/xen/xend_internal.c   |  2 +-
>  src/xenconfig/xen_sxpr.c  |  2 +-
>  tests/commandtest.c   | 10 +-
>  tests/securityselinuxlabeltest.c  |  2 +-
>  tests/virauthconfigtest.c |  2 +-
>  tests/virbitmaptest.c | 12 ++--
>  tests/vircgrouptest.c |  2 +-
>  tests/virkeyfiletest.c|  8 
>  tests/virnetsockettest.c  |  2 +-
>  tests/virtypedparamtest.c |  2 +-
>  tests/viruritest.c| 16 
>  28 files changed, 56 insertions(+), 56 deletions(-)
> 
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index d44cf2c..e5d56e9 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -197,7 +197,7 @@ bhyveConnectOpen(virConnectPtr conn,
>   if (conn->uri->server)
>   return VIR_DRV_OPEN_DECLINED;
> 
> - if (!STREQ_NULLABLE(conn->uri->path, "/system")) {
> + if (STRNEQ_NULLABLE(conn->uri->path, "/system")) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Unexpected bhyve URI path '%s', try
> bhyve:///system"),
> conn->uri->path);


This patch seems corrupt, can you please resend using 'git send-email'?
Also, it would be good if you can merge those two patches into a single
one. It's not necessary though.

Michal

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

[libvirt] [PATCH] bhyve: implement domainGetOSType

2015-10-20 Thread Roman Bogorodskiy 
---
 src/bhyve/bhyve_driver.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index d44cf2c..efba0ae 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -465,6 +465,27 @@ bhyveDomainIsPersistent(virDomainPtr domain)
 }
 
 static char *
+bhyveDomainGetOSType(virDomainPtr dom)
+{
+virDomainObjPtr vm;
+char *ret = NULL;
+
+if (!(vm = bhyveDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetOSTypeEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (VIR_STRDUP(ret, virDomainOSTypeToString(vm->def->os.type)) < 0)
+goto cleanup;
+
+ cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
+static char *
 bhyveDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
 {
 virDomainObjPtr vm;
@@ -1487,6 +1508,7 @@ static virHypervisorDriver bhyveHypervisorDriver = {
 .domainDefineXML = bhyveDomainDefineXML, /* 1.2.2 */
 .domainDefineXMLFlags = bhyveDomainDefineXMLFlags, /* 1.2.12 */
 .domainUndefine = bhyveDomainUndefine, /* 1.2.2 */
+.domainGetOSType = bhyveDomainGetOSType, /* 1.2.21 */
 .domainGetXMLDesc = bhyveDomainGetXMLDesc, /* 1.2.2 */
 .domainIsActive = bhyveDomainIsActive, /* 1.2.2 */
 .domainIsPersistent = bhyveDomainIsPersistent, /* 1.2.2 */
-- 
2.6.1

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

[libvirt] [PATCH 2/4] qemu: hostdev: Unify naming for qemuDomainReAttachHost*Devices()

2015-10-20 Thread Andrea Bolognani 
No functional changes.
---
 src/qemu/qemu_hostdev.c | 4 ++--
 src/qemu/qemu_hostdev.h | 2 +-
 src/qemu/qemu_hotplug.c | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index a4409d6..ec708c8 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -309,7 +309,7 @@ qemuPrepareHostDevices(virQEMUDriverPtr driver,
 }
 
 void
-qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
+qemuDomainReAttachHostPCIDevices(virQEMUDriverPtr driver,
  const char *name,
  virDomainHostdevDefPtr *hostdevs,
  int nhostdevs)
@@ -366,7 +366,7 @@ qemuDomainReAttachHostDevices(virQEMUDriverPtr driver,
 if (!def->nhostdevs)
 return;
 
-qemuDomainReAttachHostdevDevices(driver, def->name, def->hostdevs,
+qemuDomainReAttachHostPCIDevices(driver, def->name, def->hostdevs,
  def->nhostdevs);
 
 qemuDomainReAttachHostUSBDevices(driver, def->name, def->hostdevs,
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index af53095..156ffdd 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -65,7 +65,7 @@ void qemuDomainReAttachHostSCSIDevices(virQEMUDriverPtr 
driver,
const char *name,
virDomainHostdevDefPtr *hostdevs,
int nhostdevs);
-void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
+void qemuDomainReAttachHostPCIDevices(virQEMUDriverPtr driver,
   const char *name,
   virDomainHostdevDefPtr *hostdevs,
   int nhostdevs);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7083abf..aea6019 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1383,7 +1383,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
 if (releaseaddr)
 qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL);
 
-qemuDomainReAttachHostdevDevices(driver, vm->def->name, , 1);
+qemuDomainReAttachHostPCIDevices(driver, vm->def->name, , 1);
 
 VIR_FREE(devstr);
 VIR_FREE(configfd_name);
@@ -2968,7 +2968,7 @@ qemuDomainRemovePCIHostDevice(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   virDomainHostdevDefPtr hostdev)
 {
-qemuDomainReAttachHostdevDevices(driver, vm->def->name, , 1);
+qemuDomainReAttachHostPCIDevices(driver, vm->def->name, , 1);
 qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL);
 }
 
-- 
2.4.3

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

Re: [libvirt] [PATCH 06/10] qemu: command: Move dimm device checks from formatter to checker

2015-10-20 Thread John Ferlan 


On 10/16/2015 08:11 AM, Peter Krempa wrote:
> Aggregate the checks of the dimm device into the verification function
> rather than having them in the formatter.
> ---
>  src/qemu/qemu_command.c | 65 ++
>  src/qemu/qemu_command.h |  4 +--
>  src/qemu/qemu_domain.c  | 83 
> -
>  src/qemu/qemu_hotplug.c |  2 +-
>  4 files changed, 86 insertions(+), 68 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4a709db..5e7b052 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5318,44 +5318,8 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr 
> mem,
>  }
> 
> 
> -static bool
> -qemuCheckMemoryDimmConflict(virDomainDefPtr def,
> -virDomainMemoryDefPtr mem)
> -{
> -size_t i;
> -
> -for (i = 0; i < def->nmems; i++) {
> - virDomainMemoryDefPtr tmp = def->mems[i];
> -
> - if (tmp == mem ||
> - tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM)
> - continue;
> -
> - if (mem->info.addr.dimm.slot == tmp->info.addr.dimm.slot) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -_("memory device slot '%u' is already being "
> -  "used by another memory device"),
> -mem->info.addr.dimm.slot);
> - return true;
> - }
> -
> - if (mem->info.addr.dimm.base != 0 &&
> - mem->info.addr.dimm.base == tmp->info.addr.dimm.base) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -_("memory device base '0x%llx' is already being "
> -  "used by another memory device"),
> -mem->info.addr.dimm.base);
> - return true;
> - }
> -}
> -
> -return false;
> -}
> -
>  char *
> -qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
> - virDomainDefPtr def,
> - virQEMUCapsPtr qemuCaps)
> +qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem)
>  {
>  virBuffer buf = VIR_BUFFER_INITIALIZER;
> 
> @@ -5367,35 +5331,10 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
> 
>  switch ((virDomainMemoryModel) mem->model) {
>  case VIR_DOMAIN_MEMORY_MODEL_DIMM:
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("this qemu doesn't support the pc-dimm 
> device"));
> -return NULL;
> -}
> -
> -if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM &&
> -mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("only 'dimm' addresses are supported for the "
> - "pc-dimm device"));
> -return NULL;
> -}
> -
> -if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM &&
> -mem->info.addr.dimm.slot >= def->mem.memory_slots) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -   _("memory device slot '%u' exceeds slots count 
> '%u'"),
> -   mem->info.addr.dimm.slot, def->mem.memory_slots);
> -return NULL;
> -}
> -
>  virBufferAsprintf(, "pc-dimm,node=%d,memdev=mem%s,id=%s",
>mem->targetNode, mem->info.alias, mem->info.alias);
> 
>  if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {
> -if (qemuCheckMemoryDimmConflict(def, mem))
> -return NULL;
> -
>  virBufferAsprintf(, ",slot=%d", mem->info.addr.dimm.slot);
>  virBufferAsprintf(, ",addr=%llu", mem->info.addr.dimm.base);
>  }
> @@ -9486,7 +9425,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>qemuCaps, cfg)))
>  goto error;
> 
> -if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], def, 
> qemuCaps))) {
> +if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i]))) {
>  VIR_FREE(backStr);
>  goto error;
>  }
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 4aa7f2d..c7ed300 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -173,9 +173,7 @@ int qemuBuildMemoryBackendStr(unsigned long long size,
>virJSONValuePtr *backendProps,
>bool force);
> 
> -char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
> -   virDomainDefPtr def,
> -   virQEMUCapsPtr qemuCaps);
> +char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem);
> 
>  /* Legacy, pre device support */
>  char

[libvirt] [PATCHv4 2/2] netlink: add support for multi-part netlink messages.

2015-10-20 Thread Maxim Perevedentsev 
Such messages do not have NLMSG_ERROR or NLMSG_DONE type
but they are valid responses. We test 'multi-partness'
by looking for NLM_F_MULTI flag.
---
Difference to v1: fixed comment style.

 src/util/virnetlink.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0276522..679b48e 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -394,7 +394,9 @@ virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int 
recvbuflen)
 break;

 default:
-goto malformed_resp;
+/* We allow multipart messages. */
+if (!(resp->nlmsg_flags & NLM_F_MULTI))
+goto malformed_resp;
 }

 return result;
--
1.8.3.1

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

[libvirt] [PATCHv4 0/2] Added waiting for DAD to finish for bridge address.

2015-10-20 Thread Maxim Perevedentsev 
This is a fix for commit db488c79173b240459c7754f38c3c6af9b432970
dnsmasq main process which is relied on when waiting for DAD to complete
exits without actually waiting for DAD. This is dnsmasq daemon's task.

It seems to be a race that DAD finished before dnsmasq main process exited.
The above commit needs the execution to block until DAD finishes
for bridge IPv6 address because then it closes dummy tap device.
Thus we need to ensure this ourselves.

So we periodically poll the kernel using netlink and
check whether there are any IPv6 addresses assigned to bridge
which have 'tentative' state. After DAD is finished, execution continues.
I guess that is what dnsmasq was assumed to do.

We use netlink to dump information about existing IPv6 addresses. Netlink's
response is a multi-part message. Unfortunately, the current implementation
of virNetlink treats such messages as faulty and throws an error. So the patch 
2/2
adds multi-part nelink response support.

Update v2: fixed syntax.
Update v3: moved to virnetdev.
Update v4: added DAD timeout, minor fixes.

Maxim Perevedentsev (2):
  network: added waiting for DAD to finish for bridge address.
  netlink: add support for multi-part netlink messages.

 src/libvirt_private.syms|   1 +
 src/network/bridge_driver.c |  34 +-
 src/util/virnetdev.c| 110 
 src/util/virnetdev.h|   2 +
 src/util/virnetlink.c   |   4 +-
 5 files changed, 149 insertions(+), 2 deletions(-)

--
1.8.3.1

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

Re: [libvirt] [PATCH 0/3] NEWS: Split releases by year

2015-10-20 Thread Martin Kletzander 

On Fri, Oct 16, 2015 at 10:20:56AM +0200, Andrea Bolognani wrote:

On Fri, 2015-10-16 at 09:47 +0200, Martin Kletzander wrote:

On Thu, Oct 15, 2015 at 04:12:07PM +0200, Andrea Bolognani wrote:
> As agreed, I've followed up with the cleanups by splitting
> the huge news.html.in file into separate, smaller files,
> one per year.
>
> Along with the split I've also included a fixed XSLT
> stylesheet so that we can start shipping a meaningful
> NEWS file again.

I like it, it looks nice, there are just two issues here:

 1) the NEWS file is not generated if I run 'make all', I don't know
whether that should or should not happen, so feel free to debate
me on that one,


This was the case even before my changes and I don't
think it's an issue, as the file gets generated during
dist and ends up in the release archive.



Well, since that's pre-existing (which I haven't noticed), it
shouldn't stop us from pushing this.


Let me know if you think otherwise.


 2) syntax-check fails with this series, but that's not because there
would be anything wrong with the files you've added, it's just an
exclusion rule missing them.


Thanks for spotting that, I will include your fix.



Then ACK with that fix then.


Cheers.

--
Andrea Bolognani
Software Engineer - Virtualization Team

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


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

[libvirt] [PATCH 0/4] qemu: hostdev: Unify naming

2015-10-20 Thread Andrea Bolognani 
I was working on the QEMU hostdev code and the inconsistent
naming used kept making things just that little harder for
me to follow, so I unified it.

Cheers.


Andrea Bolognani (4):
  qemu: hostdev: Unify naming for qemuPrepareHost*Devices()
  qemu: hostdev: Unify naming for qemuDomainReAttachHost*Devices()
  qemu: hostdev: Unify naming for qemuUpdateActiveHost*Devices()
  qemu: hostdev: Introduce qemuUpdateActiveHostDevices()

 src/qemu/qemu_hostdev.c | 74 ++---
 src/qemu/qemu_hostdev.h | 36 
 src/qemu/qemu_hotplug.c | 13 -
 src/qemu/qemu_process.c |  8 +-
 4 files changed, 72 insertions(+), 59 deletions(-)

-- 
2.4.3

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

[libvirt] [PATCH 4/4] qemu: hostdev: Introduce qemuUpdateActiveHostDevices()

2015-10-20 Thread Andrea Bolognani 
This calls the PCI-, USB- and SCSI-specific functions just like
qemuPrepareHostDevices() and qemuDomainReAttachHostDevices()
already do.

Update qemuProcessReconnect() to use the new function.
---
 src/qemu/qemu_hostdev.c | 18 ++
 src/qemu/qemu_hostdev.h |  2 ++
 src/qemu/qemu_process.c |  8 +---
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index d395271..fad5975 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -83,6 +83,24 @@ qemuUpdateActiveHostSCSIDevices(virQEMUDriverPtr driver,
  QEMU_DRIVER_NAME, def->name);
 }
 
+int
+qemuUpdateActiveHostDevices(virQEMUDriverPtr driver,
+virDomainDefPtr def)
+{
+if (!def->nhostdevs)
+return 0;
+
+if (qemuUpdateActiveHostPCIDevices(driver, def) < 0)
+return -1;
+
+if (qemuUpdateActiveHostUSBDevices(driver, def) < 0)
+return -1;
+
+if (qemuUpdateActiveHostSCSIDevices(driver, def) < 0)
+return -1;
+
+return 0;
+}
 
 bool
 qemuHostdevHostSupportsPassthroughVFIO(void)
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 3565a44..e6a19ff 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -33,6 +33,8 @@ int qemuUpdateActiveHostUSBDevices(virQEMUDriverPtr driver,
virDomainDefPtr def);
 int qemuUpdateActiveHostSCSIDevices(virQEMUDriverPtr driver,
 virDomainDefPtr def);
+int qemuUpdateActiveHostDevices(virQEMUDriverPtr driver,
+virDomainDefPtr def);
 bool qemuHostdevHostSupportsPassthroughLegacy(void);
 bool qemuHostdevHostSupportsPassthroughVFIO(void);
 int qemuPrepareHostPCIDevices(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 899f634..1eb1362 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3769,13 +3769,7 @@ qemuProcessReconnect(void *opaque)
 priv->agentError = true;
 }
 
-if (qemuUpdateActiveHostPCIDevices(driver, obj->def) < 0)
-goto error;
-
-if (qemuUpdateActiveHostUSBDevices(driver, obj->def) < 0)
-goto error;
-
-if (qemuUpdateActiveHostSCSIDevices(driver, obj->def) < 0)
+if (qemuUpdateActiveHostDevices(driver, obj->def) < 0)
 goto error;
 
 if (qemuConnectCgroup(driver, obj) < 0)
-- 
2.4.3

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

[libvirt] [PATCH 1/4] qemu: hostdev: Unify naming for qemuPrepareHost*Devices()

2015-10-20 Thread Andrea Bolognani 
No functional changes.
---
 src/qemu/qemu_hostdev.c | 40 
 src/qemu/qemu_hostdev.h | 22 +++---
 src/qemu/qemu_hotplug.c |  9 -
 3 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 706db0c..a4409d6 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -156,9 +156,9 @@ qemuHostdevHostSupportsPassthroughLegacy(void)
 
 
 static bool
-qemuPrepareHostdevPCICheckSupport(virDomainHostdevDefPtr *hostdevs,
-  size_t nhostdevs,
-  virQEMUCapsPtr qemuCaps)
+qemuPrepareHostPCIDevicesCheckSupport(virDomainHostdevDefPtr *hostdevs,
+  size_t nhostdevs,
+  virQEMUCapsPtr qemuCaps)
 {
 bool supportsPassthroughKVM = qemuHostdevHostSupportsPassthroughLegacy();
 bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO();
@@ -219,18 +219,18 @@ qemuPrepareHostdevPCICheckSupport(virDomainHostdevDefPtr 
*hostdevs,
 }
 
 int
-qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
- const char *name,
- const unsigned char *uuid,
- virDomainHostdevDefPtr *hostdevs,
- int nhostdevs,
- virQEMUCapsPtr qemuCaps,
- unsigned int flags)
+qemuPrepareHostPCIDevices(virQEMUDriverPtr driver,
+  const char *name,
+  const unsigned char *uuid,
+  virDomainHostdevDefPtr *hostdevs,
+  int nhostdevs,
+  virQEMUCapsPtr qemuCaps,
+  unsigned int flags)
 {
 int ret = -1;
 virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
 
-if (!qemuPrepareHostdevPCICheckSupport(hostdevs, nhostdevs, qemuCaps))
+if (!qemuPrepareHostPCIDevicesCheckSupport(hostdevs, nhostdevs, qemuCaps))
 goto out;
 
 ret = virHostdevPreparePCIDevices(hostdev_mgr, QEMU_DRIVER_NAME,
@@ -254,10 +254,10 @@ qemuPrepareHostUSBDevices(virQEMUDriverPtr driver,
 }
 
 int
-qemuPrepareHostdevSCSIDevices(virQEMUDriverPtr driver,
-  const char *name,
-  virDomainHostdevDefPtr *hostdevs,
-  int nhostdevs)
+qemuPrepareHostSCSIDevices(virQEMUDriverPtr driver,
+   const char *name,
+   virDomainHostdevDefPtr *hostdevs,
+   int nhostdevs)
 {
 size_t i;
 virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
@@ -292,17 +292,17 @@ qemuPrepareHostDevices(virQEMUDriverPtr driver,
 if (!def->nhostdevs)
 return 0;
 
-if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid,
- def->hostdevs, def->nhostdevs,
- qemuCaps, flags) < 0)
+if (qemuPrepareHostPCIDevices(driver, def->name, def->uuid,
+  def->hostdevs, def->nhostdevs,
+  qemuCaps, flags) < 0)
 return -1;
 
 if (qemuPrepareHostUSBDevices(driver, def->name,
   def->hostdevs, def->nhostdevs, flags) < 0)
 return -1;
 
-if (qemuPrepareHostdevSCSIDevices(driver, def->name,
-  def->hostdevs, def->nhostdevs) < 0)
+if (qemuPrepareHostSCSIDevices(driver, def->name,
+   def->hostdevs, def->nhostdevs) < 0)
 return -1;
 
 return 0;
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 05bd965..af53095 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -35,23 +35,23 @@ int qemuUpdateActiveSCSIHostdevs(virQEMUDriverPtr driver,
  virDomainDefPtr def);
 bool qemuHostdevHostSupportsPassthroughLegacy(void);
 bool qemuHostdevHostSupportsPassthroughVFIO(void);
-int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
- const char *name,
- const unsigned char *uuid,
- virDomainHostdevDefPtr *hostdevs,
- int nhostdevs,
- virQEMUCapsPtr qemuCaps,
- unsigned int flags);
+int qemuPrepareHostPCIDevices(virQEMUDriverPtr driver,
+  const char *name,
+  const unsigned char *uuid,
+  virDomainHostdevDefPtr *hostdevs,
+  int nhostdevs,
+  virQEMUCapsPtr qemuCaps,
+  unsigned int flags);
 int
 qemuPrepareHostUSBDevices(virQEMUDriverPtr driver,

[libvirt] [PATCH 3/4] qemu: hostdev: Unify naming for qemuUpdateActiveHost*Devices()

2015-10-20 Thread Andrea Bolognani 
No functional changes.
---
 src/qemu/qemu_hostdev.c | 12 ++--
 src/qemu/qemu_hostdev.h | 12 ++--
 src/qemu/qemu_process.c |  6 +++---
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index ec708c8..d395271 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -45,8 +45,8 @@ VIR_LOG_INIT("qemu.qemu_hostdev");
 
 
 int
-qemuUpdateActivePCIHostdevs(virQEMUDriverPtr driver,
-virDomainDefPtr def)
+qemuUpdateActiveHostPCIDevices(virQEMUDriverPtr driver,
+   virDomainDefPtr def)
 {
 virHostdevManagerPtr mgr = driver->hostdevMgr;
 
@@ -58,8 +58,8 @@ qemuUpdateActivePCIHostdevs(virQEMUDriverPtr driver,
 }
 
 int
-qemuUpdateActiveUSBHostdevs(virQEMUDriverPtr driver,
-virDomainDefPtr def)
+qemuUpdateActiveHostUSBDevices(virQEMUDriverPtr driver,
+   virDomainDefPtr def)
 {
 virHostdevManagerPtr mgr = driver->hostdevMgr;
 
@@ -71,8 +71,8 @@ qemuUpdateActiveUSBHostdevs(virQEMUDriverPtr driver,
 }
 
 int
-qemuUpdateActiveSCSIHostdevs(virQEMUDriverPtr driver,
- virDomainDefPtr def)
+qemuUpdateActiveHostSCSIDevices(virQEMUDriverPtr driver,
+virDomainDefPtr def)
 {
 virHostdevManagerPtr mgr = driver->hostdevMgr;
 
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 156ffdd..3565a44 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -27,12 +27,12 @@
 # include "qemu_conf.h"
 # include "domain_conf.h"
 
-int qemuUpdateActivePCIHostdevs(virQEMUDriverPtr driver,
-virDomainDefPtr def);
-int qemuUpdateActiveUSBHostdevs(virQEMUDriverPtr driver,
-virDomainDefPtr def);
-int qemuUpdateActiveSCSIHostdevs(virQEMUDriverPtr driver,
- virDomainDefPtr def);
+int qemuUpdateActiveHostPCIDevices(virQEMUDriverPtr driver,
+   virDomainDefPtr def);
+int qemuUpdateActiveHostUSBDevices(virQEMUDriverPtr driver,
+   virDomainDefPtr def);
+int qemuUpdateActiveHostSCSIDevices(virQEMUDriverPtr driver,
+virDomainDefPtr def);
 bool qemuHostdevHostSupportsPassthroughLegacy(void);
 bool qemuHostdevHostSupportsPassthroughVFIO(void);
 int qemuPrepareHostPCIDevices(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 07e41f0..899f634 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3769,13 +3769,13 @@ qemuProcessReconnect(void *opaque)
 priv->agentError = true;
 }
 
-if (qemuUpdateActivePCIHostdevs(driver, obj->def) < 0)
+if (qemuUpdateActiveHostPCIDevices(driver, obj->def) < 0)
 goto error;
 
-if (qemuUpdateActiveUSBHostdevs(driver, obj->def) < 0)
+if (qemuUpdateActiveHostUSBDevices(driver, obj->def) < 0)
 goto error;
 
-if (qemuUpdateActiveSCSIHostdevs(driver, obj->def) < 0)
+if (qemuUpdateActiveHostSCSIDevices(driver, obj->def) < 0)
 goto error;
 
 if (qemuConnectCgroup(driver, obj) < 0)
-- 
2.4.3

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

Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo

2015-10-20 Thread Serge Hallyn 
Just a one-time announcement - beside the git tree at
github.com/hallyn/libresource there is also a mailing list now at
https://lists.linuxcontainers.org/listinfo/libresource-devel

I don't really intend to be a driving developer on it, but will
happily review and discuss and help where I can.

-serge

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

Re: [libvirt] [PATCH 07/10] qemu: domain: Remove memory device check from post parse callback

2015-10-20 Thread John Ferlan 


On 10/16/2015 08:11 AM, Peter Krempa wrote:
> We check that  is present when we have memory devices at the
> time we start the VM or add the device, so we can remove it from the
> post parse callback to be more tolerant to users.
> ---
>  src/qemu/qemu_domain.c | 8 
>  1 file changed, 8 deletions(-)
> 

Hmm.. we never had a DO_TEST_PARSE_FAILURE test on this...

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 1474a5b..a22e5ad 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1347,14 +1347,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>  }
>  }
> 
> -if (dev->type == VIR_DOMAIN_DEVICE_MEMORY &&
> -!virDomainDefHasMemoryHotplug(def)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("maxMemory has to be specified when using memory "
> - "devices "));

Well at least I know where you got that error message in patch 5... it
almost feels as if this could be merged there to make it more obvious.

ACK - regardless of whether it's moved or not.

John
> -goto cleanup;
> -}
> -
>  ret = 0;
> 
>   cleanup:
> 

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

[libvirt] [PATCH v2] use STRNEQ instead of !STREQ and new syntax-check rule to avoid !STREQ and !STRNEQ

2015-10-20 Thread Ishmanpreet Kaur Khera 
used STRNEQ instead of !STREQ to remove inconsistency and 
intoduced new syntax-check rule for !STREQ and !STRNEQ so
that we don't end up in the same situation again. 

Signed-off-by: Ishmanpreet Kaur Khera 
---
 cfg.mk| 18 ++
 src/bhyve/bhyve_driver.c  |  2 +-
 src/conf/network_conf.c   |  4 ++--
 src/conf/nwfilter_conf.c  |  2 +-
 src/conf/nwfilter_params.c|  2 +-
 src/conf/storage_conf.c   |  2 +-
 src/lxc/lxc_fuse.c|  4 ++--
 src/openvz/openvz_driver.c|  2 +-
 src/qemu/qemu_command.c   |  6 +++---
 src/qemu/qemu_domain.c|  2 +-
 src/qemu/qemu_hotplug.c   |  2 +-
 src/security/security_manager.c   |  2 +-
 src/security/security_selinux.c   | 12 ++--
 src/storage/storage_backend_logical.c |  2 +-
 src/util/virfile.c|  2 +-
 src/util/virsystemd.c |  2 +-
 src/vz/vz_driver.c|  2 +-
 src/vz/vz_sdk.c   |  2 +-
 src/xen/xend_internal.c   |  2 +-
 src/xenconfig/xen_sxpr.c  |  2 +-
 tests/commandtest.c   | 10 +-
 tests/securityselinuxlabeltest.c  |  2 +-
 tests/virauthconfigtest.c |  2 +-
 tests/virbitmaptest.c | 12 ++--
 tests/vircgrouptest.c |  2 +-
 tests/virkeyfiletest.c|  8 
 tests/virnetsockettest.c  |  2 +-
 tests/virtypedparamtest.c |  2 +-
 tests/viruritest.c| 16 
 29 files changed, 74 insertions(+), 56 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index e436434..7343dfc 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1011,6 +1011,18 @@ sc_prohibit_pthread_create:
halt="avoid using 'pthread_create', use 'virThreadCreate' instead" \
  $(_sc_search_regexp)
 
+sc_prohibit_not_streq:
+   @prohibit='!STREQ'\
+   exclude='exempt from syntax-check'\
+   halt='Use STRNEQ instead of !STREQ'\
+ $(_sc_search_regexp)
+
+sc_prohibit_not_strneq:
+   @prohibit='!STRNEQ'\
+   exclude='exempt from syntax-check'\
+   halt='Use STREQ instead of !STRNEQ'\
+ $(_sc_search_regexp)
+
 # We don't use this feature of maint.mk.
 prev_version_file = /dev/null
 
@@ -1213,3 +1225,9 @@ exclude_file_name_regexp--sc_prohibit_sysconf_pagesize = \
 
 exclude_file_name_regexp--sc_prohibit_pthread_create = \
   ^(cfg\.mk|src/util/virthread\.c|tests/.*)$$
+
+exclude_file_name_regexp--sc_prohibit_not_streq = \
+  ^tests/.*\.[ch]$$
+
+exclude_file_name_regexp--sc_prohibit_not_strneq = \
+  ^tests/.*\.[ch]$$
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index d44cf2c..e5d56e9 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -197,7 +197,7 @@ bhyveConnectOpen(virConnectPtr conn,
  if (conn->uri->server)
  return VIR_DRV_OPEN_DECLINED;
 
- if (!STREQ_NULLABLE(conn->uri->path, "/system")) {
+ if (STRNEQ_NULLABLE(conn->uri->path, "/system")) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unexpected bhyve URI path '%s', try 
bhyve:///system"),
conn->uri->path);
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index c1cbd76..0ffb325 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3042,7 +3042,7 @@ virNetworkLoadState(virNetworkObjListPtr nets,
 if (!(def = virNetworkDefParseXML(ctxt)))
 goto error;
 
-if (!STREQ(name, def->name)) {
+if (STRNEQ(name, def->name)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Network config filename '%s'"
  " does not match network name '%s'"),
@@ -3152,7 +3152,7 @@ virNetworkObjPtr 
virNetworkLoadConfig(virNetworkObjListPtr nets,
 if (!(def = virNetworkDefParseFile(configFile)))
 goto error;
 
-if (!STREQ(name, def->name)) {
+if (STRNEQ(name, def->name)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Network config filename '%s'"
  " does not match network name '%s'"),
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index f7ccb75..ced8eb8 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -3082,7 +3082,7 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 nwfilter = virNWFilterObjFindByUUID(nwfilters, def->uuid);
 
 if (nwfilter) {
-if (!STREQ(def->name, nwfilter->def->name)) {
+if (STRNEQ(def->name, nwfilter->def->name)) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("filter with same UUID but different name "
  "('%s') already exists"),
diff --git a/src/conf/nwfilter_params.c

Re: [libvirt] [PATCH 0/3] NEWS: Split releases by year

2015-10-20 Thread Andrea Bolognani 
On Tue, 2015-10-20 at 17:21 +0200, Martin Kletzander wrote:
> > >  2) syntax-check fails with this series, but that's not because
> > > there
> > > would be anything wrong with the files you've added, it's
> > > just an
> > > exclusion rule missing them.
> > 
> > Thanks for spotting that, I will include your fix.
> > 
> 
> Then ACK with that fix then.

Pushed, thanks :)

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

[libvirt] [PATCHv4 1/2] network: added waiting for DAD to finish for bridge address.

2015-10-20 Thread Maxim Perevedentsev 
This is a fix for commit db488c79173b240459c7754f38c3c6af9b432970
dnsmasq main process exits without waiting for DAD, this is dnsmasq
daemon's task. So we periodically poll the kernel using netlink and
check whether there are any IPv6 addresses assigned to bridge
which have 'tentative' state. After DAD is finished, execution continues.
I guess that is what dnsmasq was assumed to do.
---

Difference to v1: fixed syntax (comments and alignment).
Difference to v2: moved to virnetdev.
Difference to v3: added timeout.

 src/libvirt_private.syms|   1 +
 src/network/bridge_driver.c |  33 +-
 src/util/virnetdev.c| 109 
 src/util/virnetdev.h|   2 +
 4 files changed, 144 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index be6ee19..578c6fe 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1792,6 +1792,7 @@ virNetDevSetRcvMulti;
 virNetDevSetupControl;
 virNetDevSysfsFile;
 virNetDevValidateConfig;
+virNetDevWaitDadFinish;


 # util/virnetdevbandwidth.h
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index cd9a51e..63efffa 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2026,6 +2026,31 @@ networkAddRouteToBridge(virNetworkObjPtr network,
 }

 static int
+networkWaitDadFinish(virNetworkObjPtr network)
+{
+virNetworkIpDefPtr ipdef;
+virSocketAddrPtr *addrs = NULL, addr = NULL;
+size_t naddrs = 0;
+int ret = -1;
+
+VIR_DEBUG("Started waiting for IPv6 DAD");
+
+while ((ipdef = virNetworkDefGetIpByIndex(
+network->def, AF_INET6, naddrs))) {
+addr = >address;
+if (VIR_APPEND_ELEMENT_COPY(addrs, naddrs, addr) < 0)
+goto cleanup;
+}
+
+ret = (naddrs == 0) ? 0 : virNetDevWaitDadFinish(addrs, naddrs);
+
+ cleanup:
+VIR_FREE(addrs);
+VIR_DEBUG("IPv6 DAD finished with status %d", ret);
+return ret;
+}
+
+static int
 networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
virNetworkObjPtr network)
 {
@@ -2159,7 +2184,13 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr 
driver,
 if (v6present && networkStartRadvd(driver, network) < 0)
 goto err4;

-/* DAD has happened (dnsmasq waits for it), dnsmasq is now bound to the
+/* dnsmasq main process does not wait for DAD to complete,
+ * so we need to wait for it ourselves.
+ */
+if (v6present && networkWaitDadFinish(network) < 0)
+goto err4;
+
+/* DAD has happened, dnsmasq is now bound to the
  * bridge's IPv6 address, so we can now set the dummy tun down.
  */
 if (tapfd >= 0) {
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 579ff3b..6b6bba0 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -96,6 +96,7 @@ VIR_LOG_INIT("util.netdev");
 # define FEATURE_BIT_IS_SET(blocks, index, field)\
 (FEATURE_WORD(blocks, index, field) & FEATURE_FIELD_FLAG(index))
 #endif
+# define DAD_WAIT_TIMEOUT 5 /* seconds */

 typedef enum {
 VIR_MCAST_TYPE_INDEX_TOKEN,
@@ -1305,6 +1306,105 @@ int virNetDevClearIPAddress(const char *ifname,
 return ret;
 }

+/* return whether there is a known address with 'tentative' flag set */
+static int
+virNetDevParseDadStatus(struct nlmsghdr *nlh, int len,
+virSocketAddrPtr *addrs, size_t count)
+{
+struct ifaddrmsg *ifaddrmsg_ptr;
+unsigned int ifaddrmsg_len;
+struct rtattr *rtattr_ptr;
+size_t i;
+struct in6_addr *addr;
+for (; NLMSG_OK(nlh, len); nlh = NLMSG_NEXT(nlh, len)) {
+if (NLMSG_PAYLOAD(nlh, 0) < sizeof(struct ifaddrmsg)) {
+/* Message without payload is the last one. */
+break;
+}
+
+ifaddrmsg_ptr = (struct ifaddrmsg *)NLMSG_DATA(nlh);
+if (!(ifaddrmsg_ptr->ifa_flags & IFA_F_TENTATIVE)) {
+/* Not tentative: we are not interested in this entry. */
+continue;
+}
+
+ifaddrmsg_len = IFA_PAYLOAD(nlh);
+rtattr_ptr = (struct rtattr *) IFA_RTA(ifaddrmsg_ptr);
+for (; RTA_OK(rtattr_ptr, ifaddrmsg_len);
+rtattr_ptr = RTA_NEXT(rtattr_ptr, ifaddrmsg_len)) {
+if (RTA_PAYLOAD(rtattr_ptr) != sizeof(struct in6_addr)) {
+/* No address: ignore. */
+continue;
+}
+
+/* We check only known addresses. */
+for (i = 0; i < count; i++) {
+addr = [i]->data.inet6.sin6_addr;
+if (!memcmp(addr, RTA_DATA(rtattr_ptr),
+sizeof(struct in6_addr))) {
+/* We found matching tentative address. */
+return 1;
+}
+}
+}
+}
+return 0;
+}
+
+/* return after DAD finishes for all known IPv6 addresses or an error */
+int
+virNetDevWaitDadFinish(virSocketAddrPtr *addrs, size_t count)
+{
+

Re: [libvirt] [PATCH v2 7/12] storage: Refactor and fix virStorageBackendCreateExecCommand

2015-10-20 Thread John Ferlan 

Ping or should I just resend what's left rather than 3 adjusted and
inserted patches?

Tks,

John

On 10/14/2015 02:01 PM, John Ferlan wrote:
> Refactor the code to handle the NETFS pool separately from other pool
> types. When the original code was developed (commit id 'e1f27784')
> the virCommandSetUmask did not exist (commit id '0e1a1a8c4'), so there
> was no way to set the permissions for the creation. Now that it exists,
> we can use it. The code currently fails to create a volume in a NETFS
> pool from some other volume within the pool because the chmod done after
> file creation fails.
> 
> So adjust the code to jump to cleanup once the file is successfully
> created in the NETFS pool. If it fails or for non NETFS files, just
> perform the create, chown, and chmod in order
> 
> Signed-off-by: John Ferlan 
> ---
>  src/storage/storage_backend.c | 30 --
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index a375fe0..037d6d7 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -678,7 +678,6 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr 
> pool,
>  gid_t gid;
>  uid_t uid;
>  mode_t mode;
> -bool filecreated = false;
>  int ret = -1;
>  
>  if ((pool->def->type == VIR_STORAGE_POOL_NETFS)
> @@ -690,26 +689,29 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr 
> pool,
>  
>  virCommandSetUID(cmd, vol->target.perms->uid);
>  virCommandSetGID(cmd, vol->target.perms->gid);
> +mode = (vol->target.perms->mode == (mode_t) -1 ?
> +VIR_STORAGE_DEFAULT_VOL_PERM_MODE : vol->target.perms->mode);
> +virCommandSetUmask(cmd, 0777 - mode);
>  
> -if (virCommandRun(cmd, NULL) == 0) {
> -/* command was successfully run, check if the file was created */
> -if (stat(vol->target.path, ) >= 0)
> -filecreated = true;
> +if ((ret = virCommandRun(cmd, NULL)) == 0) {
> +/* command was successfully run, if the file was created
> + * then we are done */
> +if (virFileExists(vol->target.path))
> +goto cleanup;
>  }
>  }
>  
> -/* don't change uid/gid if we retry */
> +/* don't change uid/gid/mode if we retry */
>  virCommandSetUID(cmd, -1);
>  virCommandSetGID(cmd, -1);
> +virCommandSetUmask(cmd, 0);
>  
> -if (!filecreated) {
> -if (virCommandRun(cmd, NULL) < 0)
> -goto cleanup;
> -if (stat(vol->target.path, ) < 0) {
> -virReportSystemError(errno,
> - _("failed to create %s"), vol->target.path);
> -goto cleanup;
> -}
> +if (virCommandRun(cmd, NULL) < 0)
> +goto cleanup;
> +if (stat(vol->target.path, ) < 0) {
> +virReportSystemError(errno,
> + _("failed to create %s"), vol->target.path);
> +goto cleanup;
>  }
>  
>  uid = (vol->target.perms->uid != st.st_uid) ? vol->target.perms->uid
> 

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

Re: [libvirt] [PATCH 08/10] conf: Prepare making memory device target node optional

2015-10-20 Thread John Ferlan 


On 10/16/2015 08:11 AM, Peter Krempa wrote:
> Adjust the config code so that it does not enforce that target memory
> node is specified. To avoid breakage, adjust the qemu memory hotplug
> config checker to disallow such config for now.
> ---
>  docs/formatdomain.html.in |  5 +++--
>  docs/schemas/domaincommon.rng |  8 +---
>  src/conf/domain_conf.c| 10 +++---
>  src/qemu/qemu_domain.c|  7 +++
>  4 files changed, 22 insertions(+), 8 deletions(-)
> 

It almost feels like patches 9 & 10 should precede this one. Perhaps
easier with the html.in change. Consider this as a review for 8-10 as
well as some other random thoughts.

So if I'm reading things correctly, a PPC64 guest "can" supply NUMA
guest nodes, but it doesn't have to. Is that a fair assumption? It
matters mostly to help describe things.  If it can have it, then perhaps
another test (copy qemuxml2argv-memory-hotplug-ppc64-nonuma.xml to
qemuxml2argv-memory-hotplug-ppc64.xml and add the numa node defs).

If PPC64 doesn't support NUMA guest nodes, then does it make sense in
post processing code to set targetNode = -1 if ARCH_IS_PPC64? It
wouldn't be the first PPC64 specific code.  Along the same lines, would
be ignoring targetNode in patch 9 if PPC64.  Furthermore, if PPC64
doesn't support NUMA guest nodes, I would have expected a check during
parse - I didn't look that hard for this case.  I was merely thinking
what happens if PPC64 does define NUMA guest nodes and those can be used
by the memory device.

> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index c88b032..279424d 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6300,8 +6300,9 @@ qemu-kvm -net nic,model=? /dev/null
>added memory as a scaled integer.
>  
>  
> -  The mandatory node subelement configures the guest 
> NUMA
> -  node to attach the memory to.
> +  The node subelement configures the guest NUMA node to
> +  attach the memory to. Note: Some hypervisors or specific
> +  configurations may require that node is specified.

How about:

The optional node subelement provides the guest cpu NUMA node or cell id to attach the memory.

 Note: The hypervisor may require the node to be
specified for some architectures.


FWIW: It almost feels like we need an example of what is meant - that is
"For QEMU/KVM, the PowerPC64 pseries guests do not require that the
node subelement is defined. If not defined, then the memory
device will be attached to [???]."

BTW: The additional text only makes sense as of patch 10 and it wasn't
clear if PPC64 "could" support numa guest nodes.


>  
>
>  
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f196177..994face 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4532,9 +4532,11 @@
>  
>
>  
> -
> -  
> -
> +
> +  
> +
> +  
> +
>
>  
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 514bd8a..a5ab29c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -12520,11 +12520,15 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
>  int ret = -1;
>  xmlNodePtr save = ctxt->node;
>  ctxt->node = node;
> +int rv;
> 
> -if (virXPathInt("string(./node)", ctxt, >targetNode) < 0 ||
> -def->targetNode < 0) {
> +/* initialize to value which marks that the user didn't speify it */

specify

This feels like a need for something like the Tristate{Bool|State}
structs...  Or way to force optional fields to something specific

> +def->targetNode = -1;
> +
> +if ((rv = virXPathInt("string(./node)", ctxt, >targetNode)) == -2 ||
> +(rv == 0 && def->targetNode < 0)) {
>  virReportError(VIR_ERR_XML_ERROR, "%s",
> -   _("invalid or missing value of memory device node"));
> +   _("invalid value of memory device node"));

...value for memory...

>  goto cleanup;
>  }
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index a22e5ad..29fed1d 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3591,6 +3591,13 @@ qemuDomainDefValidateMemoryHotplugDevice(const 
> virDomainMemoryDef *mem,
>  return -1;
>  }
> 
> +if (mem->targetNode == -1) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("target NUMA node needs to be specifed for 
> memory "

specified

Before an explicit ACK - a few adjustments I think would be beneficial.

John

> + "device"));
> +return -1;
> +}
> +
>  if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {
>  if (mem->info.addr.dimm.slot >= def->mem.memory_slots) {
>

Re: [libvirt] [PATCH 04/10] qemu: command: Always execute memory device formatter

2015-10-20 Thread John Ferlan 


On 10/16/2015 08:11 AM, Peter Krempa wrote:
> Since we already make sure before that the domain configuration is
> valid we may execute it always at the cost of doing 0 iterations of the
> for loop.
> 
> This patch will simplify later refactor as it will avoid whitespace
> changes.
> ---
>  src/qemu/qemu_command.c | 47 +++
>  1 file changed, 23 insertions(+), 24 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index a37a4fa..43e81c7 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9484,38 +9484,37 @@ qemuBuildCommandLine(virConnectPtr conn,
>  }
>  }
> 
> -if (virDomainNumaGetNodeCount(def->numa)) {
> -if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0)
> +if (virDomainNumaGetNodeCount(def->numa) &&
> +qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0)
>  goto error;
> 
> -/* memory hotplug requires NUMA to be enabled - we already checked
> - * that memory devices are present only when NUMA is */
> +/* memory hotplug requires NUMA to be enabled - we already checked
> + * that memory devices are present only when NUMA is */

The second half of this comment made me wonder where...  especially with
the removal of the virDomainNumaGetNodeCount call which would
essentially be checking if numa was enabled (I think).

The virDomainDefHasMemoryHotplug checks memory_slots or max_memory

The virDomainDefPostParseMemory could have it, but it's not overt

Not that the check is wrong, but it would be nice to fixup the comment
to indicate that 'xxx' checked that memory devices are present only when
NUMA is.  That at least helps ensure any future refactor doesn't impact
the assumption made here.

ACK with that

John
> 
> -if (def->nmems > def->mem.memory_slots) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -   _("memory device count '%zu' exceeds slots count 
> '%u'"),
> -   def->nmems, def->mem.memory_slots);
> -goto error;
> -}
> -
> -for (i = 0; i < def->nmems; i++) {
> -char *backStr;
> -char *dimmStr;
> -
> -if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def,
> -  qemuCaps, cfg)))
> -goto error;
> +if (def->nmems > def->mem.memory_slots) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("memory device count '%zu' exceeds slots count 
> '%u'"),
> +   def->nmems, def->mem.memory_slots);
> +goto error;
> +}
> 
> -if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], def, 
> qemuCaps))) {
> -VIR_FREE(backStr);
> -goto error;
> -}
> +for (i = 0; i < def->nmems; i++) {
> +char *backStr;
> +char *dimmStr;
> 
> -virCommandAddArgList(cmd, "-object", backStr, "-device", 
> dimmStr, NULL);
> +if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def,
> +  qemuCaps, cfg)))
> +goto error;
> 
> +if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], def, 
> qemuCaps))) {
>  VIR_FREE(backStr);
> -VIR_FREE(dimmStr);
> +goto error;
>  }
> +
> +virCommandAddArgList(cmd, "-object", backStr, "-device", dimmStr, 
> NULL);
> +
> +VIR_FREE(backStr);
> +VIR_FREE(dimmStr);
>  }
> 
>  if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_UUID))
> 

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

Re: [libvirt] [PATCH 05/10] qemu: domain: Add common function to perform memory hotplug checks

2015-10-20 Thread John Ferlan 


On 10/16/2015 08:11 AM, Peter Krempa wrote:
> Add a function that will aggregate various checks related to memory
> hotplug so that they aren't scattered accross various parts of the
> code.
> ---
>  src/qemu/qemu_command.c | 26 ++---
>  src/qemu/qemu_domain.c  | 77 
> +
>  src/qemu/qemu_domain.h  |  4 +++
>  src/qemu/qemu_hotplug.c |  9 ++
>  4 files changed, 87 insertions(+), 29 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 43e81c7..4a709db 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9414,26 +9414,12 @@ qemuBuildCommandLine(virConnectPtr conn,
>  qemuDomainAlignMemorySizes(def) < 0)
>  goto error;
> 
> +if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0)
> +goto error;
> +
>  virCommandAddArg(cmd, "-m");
> 
>  if (virDomainDefHasMemoryHotplug(def)) {
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("memory hotplug isn't supported by this QEMU 
> binary"));
> -goto error;
> -}
> -
> -/* due to guest support, qemu would silently enable NUMA with one 
> node
> - * once the memory hotplug backend is enabled. To avoid possible
> - * confusion we will enforce user originated numa configuration along
> - * with memory hotplug. */
> -if (virDomainNumaGetNodeCount(def->numa) == 0) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("At least one numa node has to be configured 
> when "
> - "enabling memory hotplug"));
> -goto error;
> -}
> -
>  /* Use the 'k' suffix to let qemu handle the units */
>  virCommandAddArgFormat(cmd, "size=%lluk,slots=%u,maxmem=%lluk",
> virDomainDefGetMemoryInitial(def),
> @@ -9491,12 +9477,6 @@ qemuBuildCommandLine(virConnectPtr conn,
>  /* memory hotplug requires NUMA to be enabled - we already checked
>   * that memory devices are present only when NUMA is */
> 

Does the comment make sense any more?

> -if (def->nmems > def->mem.memory_slots) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -   _("memory device count '%zu' exceeds slots count 
> '%u'"),
> -   def->nmems, def->mem.memory_slots);
> -goto error;
> -}
> 
>  for (i = 0; i < def->nmems; i++) {
>  char *backStr;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index bdc0e67..cb50754 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3553,6 +3553,83 @@ qemuDomainMachineIsS390CCW(const virDomainDef *def)
> 
> 
>  /**
> + * qemuDomainDefValidateMemoryHotplug:

Not necessarily called only in Hotplug path, but I understand why it's
named this way since *HasMemoryHotplug uses similar naming and adding a
Capable on the end is an unnecessarily long name.

> + * @def: domain definition
> + * @qemuCaps: qemu capabilities object
> + * @mem: definition of memory device that is to be added to @def with hotplug

or NULL when ...

> + *
> + * Validates that the domain definition and memory modules have valid
> + * configuration and are possibly able to accept @mem via hotplug if it's
> + * passed.
> + *
> + * Returns 0 on success; -1 and a libvirt error on error.
> + */
> +int
> +qemuDomainDefValidateMemoryHotplug(const virDomainDef *def,
> +   virQEMUCapsPtr qemuCaps,
> +   const virDomainMemoryDef *mem)
> +{
> +unsigned int nmems = def->nmems;
> +unsigned long long hotplugSpace;
> +unsigned long long hotplugMemory = 0;
> +size_t i;
> +
> +hotplugSpace = def->mem.max_memory - virDomainDefGetMemoryInitial(def);
> +
> +if (mem) {
> +nmems++;
> +hotplugMemory = mem->size;

I think you'd have to move qemuDomainMemoryDeviceAlignSize to sooner in
the caller so that this calculation and future check is more correct.

> +}
> +
> +if (!virDomainDefHasMemoryHotplug(def)) {
> +if (nmems) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("maxMemory has to be specified when using 
> memory "
> + "devices "));

extraneous space^

Although I have to say the check and error reads strange... The *Has
function returns true when memory_slots || max_memory > 0. So while it's
obvious knowing the code and XML that maxMemory needs to be defined in
order for hotplug to work, it just a strange message to see for someone
perhaps passing memory device XML.

Essentially there's a failure because someone was requesting to hotplug
a memory device, but the domain doesn't support it. Perhaps, "cannot
hotplug a memory device

[libvirt] [PATCHv2 0/3] improve the error report for virNodeAllocPages and virNodeGetFreePages

2015-10-20 Thread Luyao Huang 
v1:
https://www.redhat.com/archives/libvir-list/2015-September/msg01054.html

v2:
-improve the error message
-improve the code struct
-add a new patch for rework the exist way to check node and page size

Luyao Huang (3):
  util: split the virNumaGetHugePageInfoPath into separate function
  util: move the pagesize and node check and error report to one place
  util: Produce friendlier error message to user

 src/util/virnuma.c | 113 ++---
 1 file changed, 72 insertions(+), 41 deletions(-)

-- 
1.8.3.1

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

[libvirt] [PATCH 2/3] util: move the pagesize and node check and error report to one place

2015-10-20 Thread Luyao Huang 
Just like 1c24cfe9, check the pagesize and numa node if we cannot find
the hugepage path. And improve the error message again.

After this patch:
 # virsh allocpages --pagesize 2047 --pagecount 1 --cellno 0
 error: operation failed: page size 2047 is not available on node 0

 # virsh allocpages --pagesize 2047 --pagecount 1
 error: operation failed: page size 2047 is not available

Signed-off-by: Luyao Huang 
---
 src/util/virnuma.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index cb80972..815cbc1 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -836,19 +836,25 @@ virNumaSetPagePoolSize(int node,
 goto cleanup;
 }
 
-if (node != -1 && !virNumaNodeIsAvailable(node)) {
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("NUMA node %d is not available"),
-   node);
-goto cleanup;
-}
-
 if (virNumaGetHugePageInfoPath(_path, node, page_size, "nr_hugepages") 
< 0)
 goto cleanup;
 
 if (!virFileExists(nr_path)) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-   _("page size or NUMA node not available"));
+if (node != -1) {
+if (!virNumaNodeIsAvailable(node)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("NUMA node %d is not available"),
+   node);
+} else {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("page size %u is not available on node %d"),
+   page_size, node);
+}
+} else {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("page size %u is not available"),
+   page_size);
+}
 goto cleanup;
 }
 
-- 
1.8.3.1

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

[libvirt] [PATCHv2 1/3] util: split the virNumaGetHugePageInfoPath into separate function

2015-10-20 Thread Luyao Huang 
https://bugzilla.redhat.com/show_bug.cgi?id=1265114

When pass 0 page_size to virNumaGetHugePageInfoPath function, we will
get fail like this:

 error : virFileReadAll:1358 : Failed to read file 
'/sys/devices/system/node/node0/hugepages/': Is a directory

Because when the page_size is 0 the virNumaGetHugePageInfoPath will
build the directory of system path, but we don't want that.

Introduce a new helper to build the dir path could avoid this issue.

Signed-off-by: Luyao Huang 
---
 src/util/virnuma.c | 51 +++
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 1a62d62..cb80972 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -493,44 +493,31 @@ virNumaGetHugePageInfoPath(char **path,
unsigned int page_size,
const char *suffix)
 {
-
-int ret = -1;
-
 if (node == -1) {
 /* We are aiming at overall system info */
-if (page_size) {
-/* And even on specific huge page size */
-if (virAsprintf(path,
-HUGEPAGES_SYSTEM_PREFIX HUGEPAGES_PREFIX "%ukB/%s",
-page_size, suffix ? suffix : "") < 0)
-goto cleanup;
-} else {
-if (VIR_STRDUP(*path, HUGEPAGES_SYSTEM_PREFIX) < 0)
-goto cleanup;
-}
-
+return virAsprintf(path,
+   HUGEPAGES_SYSTEM_PREFIX HUGEPAGES_PREFIX "%ukB/%s",
+   page_size, suffix ? suffix : "");
 } else {
 /* We are aiming on specific NUMA node */
-if (page_size) {
-/* And even on specific huge page size */
-if (virAsprintf(path,
-HUGEPAGES_NUMA_PREFIX "node%d/hugepages/"
-HUGEPAGES_PREFIX "%ukB/%s",
-node, page_size, suffix ? suffix : "") < 0)
-goto cleanup;
-} else {
-if (virAsprintf(path,
-HUGEPAGES_NUMA_PREFIX "node%d/hugepages/",
-node) < 0)
-goto cleanup;
-}
+return virAsprintf(path,
+   HUGEPAGES_NUMA_PREFIX "node%d/hugepages/"
+   HUGEPAGES_PREFIX "%ukB/%s",
+   node, page_size, suffix ? suffix : "");
 }
-
-ret = 0;
- cleanup:
-return ret;
 }
 
+static int
+virNumaGetHugePageInfoDir(char **path, int node)
+{
+if (node == -1) {
+return VIR_STRDUP(*path, HUGEPAGES_SYSTEM_PREFIX);
+} else {
+return virAsprintf(path,
+   HUGEPAGES_NUMA_PREFIX "node%d/hugepages/",
+   node);
+}
+}
 
 /**
  * virNumaGetHugePageInfo:
@@ -724,7 +711,7 @@ virNumaGetPages(int node,
  * is always shown as used memory. Here, however, we want to report
  * slightly different information. So we take the total memory on a node
  * and subtract memory taken by the huge pages. */
-if (virNumaGetHugePageInfoPath(, node, 0, NULL) < 0)
+if (virNumaGetHugePageInfoDir(, node) < 0)
 goto cleanup;
 
 if (!(dir = opendir(path))) {
-- 
1.8.3.1

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

[libvirt] [PATCHv2 3/3] util: Produce friendlier error message to user

2015-10-20 Thread Luyao Huang 
Commit 1c24cfe9 fix the problem in virNumaSetPagePoolSize,
this patch just like it and fix the issue in another function.

when user use freepages and specify a invalid node or page size,
will get error like this:

  # virsh freepages 0 1
  error: Failed to open file 
'/sys/devices/system/node/node0/hugepages/hugepages-1kB/free_hugepages':No such 
file or directory

Add two checks to catch this and therefore produce much more
friendlier error messages.

Signed-off-by: Luyao Huang 
---
 src/util/virnuma.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 815cbc1..cd000f9 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -553,6 +553,25 @@ virNumaGetHugePageInfo(int node,
page_size, "nr_hugepages") < 0)
 goto cleanup;
 
+if (!virFileExists(path)) {
+if (node != -1) {
+if (!virNumaNodeIsAvailable(node)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("NUMA node %d is not available"),
+   node);
+} else {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("page size %u is not available on node 
%d"),
+   page_size, node);
+}
+} else {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("page size %u is not available"),
+   page_size);
+}
+goto cleanup;
+}
+
 if (virFileReadAll(path, 1024, ) < 0)
 goto cleanup;
 
@@ -572,6 +591,25 @@ virNumaGetHugePageInfo(int node,
page_size, "free_hugepages") < 0)
 goto cleanup;
 
+if (!virFileExists(path)) {
+if (node != -1) {
+if (!virNumaNodeIsAvailable(node)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("NUMA node %d is not available"),
+   node);
+} else {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("page size %u is not available on node 
%d"),
+   page_size, node);
+}
+} else {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("page size %u is not available"),
+   page_size);
+}
+goto cleanup;
+}
+
 if (virFileReadAll(path, 1024, ) < 0)
 goto cleanup;
 
-- 
1.8.3.1

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

Re: [libvirt] [PATCH 02/10] conf: Turn targetNode in struct virDomainMemoryDef to signed

2015-10-20 Thread John Ferlan 


On 10/16/2015 08:11 AM, Peter Krempa wrote:
> Later on, we will need to know whether the user specified the target
> node or not. Turn the variable into a signed value. We already treat it
> as signed in various parts of the qemu driver.
> ---
>  src/conf/domain_conf.c | 10 ++
>  src/conf/domain_conf.h |  2 +-
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 

Seeing domaincommon.rng change in patch 8 got me to thinking - should it
change here? That is from unsignedInt to integer. Especially since
you're checking the return value determined < 0.

ACK - with that adjustment.

John

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

Re: [libvirt] [PATCH 01/10] conf: Make @def const in virDomainDefGetMemoryInitial

2015-10-20 Thread John Ferlan 


On 10/16/2015 08:11 AM, Peter Krempa wrote:
> Keep const correctness and allow to use this function in cases where
> @def is const in the caller.
> ---
>  src/conf/domain_conf.c | 2 +-
>  src/conf/domain_conf.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

All because the new API you add in patch 5 uses "const virDomainDef *" -
not that it matters one way or another to me, but it would seem other
Get API's could/should have the same prototype change...  Eventually...

ACK -

John

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

Re: [libvirt] [PATCH 03/10] qemu: command: Make qemuBuildMemoryBackendStr usable without NUMA

2015-10-20 Thread John Ferlan 


On 10/16/2015 08:11 AM, Peter Krempa wrote:
> Make the function usable so that -1 can be passed to it as cell ID so
> that we can later enable memory hotplug on non-NUMA guests for certain
> architectures.
> 
> I've inspected all functions that take guestNode as an argument to
> verify that they are eiter safe to be called or are not called at all.

either

Although it seems that comment is left for under the --- ...

> ---
>  src/qemu/qemu_command.c | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f727d0b..a37a4fa 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5011,7 +5011,8 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>   * qemuBuildMemoryBackendStr:
>   * @size: size of the memory device in kibibytes
>   * @pagesize: size of the requested memory page in KiB, 0 for default
> - * @guestNode: NUMA node in the guest that the memory object will be 
> attached to
> + * @guestNode: NUMA node in the guest that the memory object will be attached
> + * to, or -1 if NUMA is not used in the guest
>   * @hostNodes: map of host nodes to alloc the memory in, NULL for default
>   * @autoNodeset: fallback nodeset in case of automatic numa placement
>   * @def: domain definition object
> @@ -5058,7 +5059,8 @@ qemuBuildMemoryBackendStr(unsigned long long size,
>  *backendType = NULL;
> 
>  /* memory devices could provide a invalid guest node */
> -if (guestNode >= virDomainNumaGetNodeCount(def->numa)) {
> +if (guestNode >= 0 &&
> +guestNode >= virDomainNumaGetNodeCount(def->numa)) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("can't add memory backend for guest node '%d' as "
>   "the guest has only '%zu' NUMA nodes configured"),
> @@ -5069,7 +5071,9 @@ qemuBuildMemoryBackendStr(unsigned long long size,
>  if (!(props = virJSONValueNewObject()))
>  return -1;

^^ this could move
> 
> -memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode);
> +if (guestNode >= 0)
> +memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, 
> guestNode);
> +
>  if (virDomainNumatuneGetMode(def->numa, guestNode, ) < 0 &&
>  virDomainNumatuneGetMode(def->numa, -1, ) < 0)
>  mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;

^^
Seems to me the code could be adjusted a bit to have:

if (guestNode >= 0) {
if (guestNode >= virDomainNumaGetNodeCount(def->numa))
...
memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa,
guestNode);
...
if (virDomainNumatuneGetMode(def->numa, guestNode, ) < 0 &&
...
}

where 'mode' is initialized to VIR_DOMAIN_NUMATUNE_MEM_STRICT;

and the props call moves either before or after the blob.


> @@ -5086,6 +5090,10 @@ qemuBuildMemoryBackendStr(unsigned long long size,
>  continue;
>  }
> 
> +/* just find the master hugepage in case we don't use NUMA */
> +if (guestNode < 0)
> +continue;
> +

So what you're say is if master_hugepage is set/found, then we don't
need to find anything else? Or can there be more than one
master_hugepage and it is desired to find the "last"?

Is there perhaps a cause/reason to break the loop rather than continue?
 IOW: Move the check inside the previous if and use it as a cause to
break the loop.

I think this is ACK-able with a couple of adjustments, but just wanted
to check what was more reasonable first - especially with this < 0 change.

John

>  if (virBitmapGetBit(hugepage->nodemask, guestNode,
>  ) < 0) {
>  /* Ignore this error. It's not an error after all. Well,
> 

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

Re: [libvirt] [PATCH 02/10] conf: Turn targetNode in struct virDomainMemoryDef to signed

2015-10-20 Thread Peter Krempa 
On Tue, Oct 20, 2015 at 09:36:29 -0400, John Ferlan wrote:
> 
> 
> On 10/16/2015 08:11 AM, Peter Krempa wrote:
> > Later on, we will need to know whether the user specified the target
> > node or not. Turn the variable into a signed value. We already treat it
> > as signed in various parts of the qemu driver.
> > ---
> >  src/conf/domain_conf.c | 10 ++
> >  src/conf/domain_conf.h |  2 +-
> >  2 files changed, 7 insertions(+), 5 deletions(-)
> > 
> 
> Seeing domaincommon.rng change in patch 8 got me to thinking - should it
> change here? That is from unsignedInt to integer. Especially since
> you're checking the return value determined < 0.

Not really. This change shouldn't change the existing semantics of the
field. It only modifies the data type but does not change what is
parsed from the user. Mixing in the domaincommon.rng change would
actually mix up the non-change part with the semantic change.

Also the rng change merely allows that this field is omitted. Also patch
8 still will be required since the semantics need to be kept until the
last patch that adds the checks.

Peter


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