[libvirt] [PATCH go-xml] 1, add support for disk cache and io, and add test case. 2, add support for controller model, and add test case. 3, extend DomainAddress struct for PCI address and target

[ZhenweiPi]

---

 domain.go  | 13 +++--
 domain_test.go | 56 +---
 2 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/domain.go b/domain.go
index 848835a..cbb22e5 100644
--- a/domain.go
+++ b/domain.go
@@ -30,8 +30,10 @@ import (
 )
 
 type DomainController struct {

-   Type  string `xml:"type,attr"`
-   Index string `xml:"index,attr"`
+   Typestring `xml:"type,attr"`
+   Index   *uint  `xml:"index,attr"`
+   Model   string `xml:"model,attr,omitempty"`
+   Address *DomainAddress `xml:"address"`
 }
 
 type DomainDiskSecret struct {

@@ -77,6 +79,8 @@ type DomainDisk struct {
Type string`xml:"type,attr"`
Device   string`xml:"device,attr"`
Snapshot string`xml:"snapshot,attr,omitempty"`
+   Cachestring`xml:"cache,attr,omitempty"`
+   Io   string`xml:"io,attr,omitempty"`
Driver   *DomainDiskDriver `xml:"driver"`
Auth *DomainDiskAuth   `xml:"auth"`
Source   *DomainDiskSource `xml:"source"`
@@ -196,8 +200,13 @@ type DomainAlias struct {
 type DomainAddress struct {
Type   string `xml:"type,attr"`
Controller *uint  `xml:"controller,attr"`
+   Domain *uint  `xml:"domain,attr"`
Bus*uint  `xml:"bus,attr"`
Port   *uint  `xml:"port,attr"`
+   Slot   *uint  `xml:"slot,attr"`
+   Function   *uint  `xml:"function,attr"`
+   Target *uint  `xml:"target,attr"`
+   Unit   *uint  `xml:"unit,attr"`
 }
 
 type DomainChardev struct {

diff --git a/domain_test.go b/domain_test.go
index 265cf80..22da947 100644
--- a/domain_test.go
+++ b/domain_test.go
@@ -30,6 +30,16 @@ import (
"testing"
 )
 
+type PciAddress struct {

+   Domain   uint
+   Bus  uint
+   Slot uint
+   Function uint
+}
+
+var uhciIndex uint = 0
+var uhciAddr = PciAddress{0, 0, 1, 2}
+
 var domainTestData = []struct {
Object   *Domain
Expected []string
@@ -130,10 +140,12 @@ var domainTestData = []struct {
},
},
DomainDisk{
-   Type: "volume",
+   Type:   "volume",
Device: "cdrom",
+   Cache:  "none",
+   Io: "native",
Source: {
-   Pool: "default",
+   Pool:   "default",
Volume: "myvolume",
},
Target: {
@@ -174,7 +186,7 @@ var domainTestData = []struct {
`  `,
`  `,
``,
-   ``,
+   ``,
`  `,
`  `,
``,
@@ -820,6 +832,44 @@ var domainTestData = []struct {
``,
},
},
+   {
+   Object: {
+   Type: "kvm",
+   Name: "test",
+   Devices: {
+   Controllers: []DomainController{
+   DomainController{
+   Type:  "usb",
+   Index: ,
+   Model: "piix3-uhci",
+   Address: {
+   Type: "pci",
+   Domain:   
,
+   Bus:  ,
+   Slot: 
,
+   Function: 
,
+   },
+   },
+   DomainController{
+   Type:  "usb",
+   Index: nil,
+   Model: "ehci",
+   },
+   },
+   },
+   },
+   Expected: []string{
+   ``,
+   `  test`,
+   `  `,
+   ``,
+   `  `,
+   ``,

Re: [libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

[Eli Qiao]

ping 


On Wednesday, 17 May 2017 at 5:08 PM, taget wrote:

> From: Eli Qiao 
> 
> * This patch amends the cache bank capability as follow:
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> For CDP enabled on x86 arch, we will have:
> 
> 
> 
> 
> 
> ...
> 
> * Added a new testdata directory `linux-resctrl-cdp` to test CDP enabled
> case.
> 
> * Along with vircaps2xmltest case updated.
> 
> P.S. here the scope is from /sys/fs/resctrl/info/L3{"" "code" "data"}, I
> keep it capital upper as I need to use a macro to convert from enum to
> string easily.
> 
> Signed-off-by: Eli Qiao 
> ---
> docs/schemas/capability.rng | 20 
> src/conf/capabilities.c | 133 -
> src/conf/capabilities.h | 10 ++
> .../vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus | 1 +
> .../linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask | 1 +
> .../resctrl/info/L3CODE/min_cbm_bits | 1 +
> .../resctrl/info/L3CODE/num_closids | 1 +
> .../linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask | 1 +
> .../resctrl/info/L3DATA/min_cbm_bits | 1 +
> .../resctrl/info/L3DATA/num_closids | 1 +
> .../linux-resctrl-cdp/resctrl/manualres/cpus | 1 +
> .../linux-resctrl-cdp/resctrl/manualres/schemata | 2 +
> .../linux-resctrl-cdp/resctrl/manualres/tasks | 0
> .../linux-resctrl-cdp/resctrl/schemata | 2 +
> .../linux-resctrl-cdp/resctrl/tasks | 0
> tests/vircaps2xmldata/linux-resctrl-cdp/system | 1 +
> .../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml | 55 +
> tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 8 +-
> tests/vircaps2xmltest.c | 8 ++
> 19 files changed, 244 insertions(+), 3 deletions(-)
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
> create mode 100755 
> tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
> create mode 100755 
> tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
> create mode 100755 
> tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
> create mode 100755 
> tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
> create mode 100755 
> tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
> create mode 100755 
> tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
> create mode 100644 
> tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
> create mode 100644 
> tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
> create mode 100644 
> tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
> create mode 12 tests/vircaps2xmldata/linux-resctrl-cdp/system
> create mode 100644 tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> 
> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> index 26f0aa2..927fc18 100644
> --- a/docs/schemas/capability.rng
> +++ b/docs/schemas/capability.rng
> @@ -277,6 +277,26 @@
> 
> 
> 
> + 
> + 
> + 
> + 
> + 
> + 
> + 
> + 
> + 
> + 
> + both
> + code
> + data
> + 
> + 
> + 
> + 
> + 
> + 
> + 
> 
> 
> 
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index d699b08..c4a1fdf 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -51,6 +51,7 @@
> #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
> 
> #define SYSFS_SYSTEM_PATH "/sys/devices/system"
> +#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl"
> 
> VIR_LOG_INIT("conf.capabilities")
> 
> @@ -872,6 +873,9 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> virCapsHostCacheBankPtr *caches)
> {
> size_t i = 0;
> + size_t j = 0;
> + int indent = virBufferGetIndent(buf, false);
> + virBuffer controlBuf = VIR_BUFFER_INITIALIZER;
> 
> if (!ncaches)
> return 0;
> @@ -893,13 +897,35 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> */
> virBufferAsprintf(buf,
> " - "size='%llu' unit='%s' cpus='%s'/>\n",
> + "size='%llu' unit='%s' cpus='%s'",
> bank->id, bank->level,
> virCacheTypeToString(bank->type),
> bank->size >> (kilos * 10),
> kilos ? "KiB" : "B",
> cpus_str);
> 
> + virBufferAdjustIndent(, indent + 4);
> + for (j = 0; j < bank->ncontrols; j++) {
> + bool min_kilos = !(bank->controls[j]->min % 1024);
> + virBufferAsprintf(,
> + " + "scope='%s' max_allocation='%u'/>\n",
> + bank->controls[j]->min >> (min_kilos * 10),
> + min_kilos ? "KiB" : "B",
> + virCacheTypeToString(bank->controls[j]->scope),
> + bank->controls[j]->max_allocation);
> + }
> +
> + if (virBufferUse()) {
> + virBufferAddLit(buf, ">\n");
> + virBufferAddBuffer(buf, );
> + virBufferAddLit(buf, "\n");
> +
> + } else {
> + virBufferAddLit(buf, "/>\n");
> + }
> +
> + virBufferFreeAndReset();
> VIR_FREE(cpus_str);
> }
> 
> @@ -1519,13 +1545,102 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
> void
> virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
> {
> + size_t i;
> +
> if (!ptr)
> return;

[libvirt] Availability of libvirt-3.4.0 Release Candidate 2

[Daniel Veillard]

  As scheduled, I have tagged it in git and pushed signed tarball and rpms
to the usual place:

  ftp://libvirt.org/libvirt/

 no difference in my limited testing compared to rc1, looks fine
https://ci.centos.org/view/libvirt/ is green except libvirt-master-build
being red which looks weird.

  There s still a couple of days to give feedback and fix eventual problems
before the release, which should hopefully take place on Thursday !

  Please give it a try,

thanks,

Daniel

-- 
Daniel Veillard  | Red Hat Developers Tools http://developer.redhat.com/
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [GSoC] Project of libvirt/qemu fuzzing

[Dan]

On Tue, May 30, 2017 at 02:08:54PM +0100, Stefan Hajnoczi wrote:
> On Tue, May 30, 2017 at 12:03 PM, Dan  wrote:
> > The project of qemu command line fuzzing has been accepted as a GSoC
> > project [1] [2]. As a student participating Google Summer of Code
> > activity, I am extremely exitited to get started today on May 30th,
> > 2017.
> 
> Welcome!  Great project idea, I am looking forward to your contributions.
> 
Thank you very much. I am very glad!
> Do you have a particular fuzzer in mind or will you write a custom
> fuzzer from scratch?
> 
I planned to come with a list of fuzzer candidates and try them all. But
now I am only playing with AFL and I would not start writing from
scratch until I know for sure what I really need to do.
So next, while I try with AFL I can start looking into fuzzers
particularly with XML grammer generation or something like that
potentially modifiable/extensible by ourselves.
> I'm not aware of anyone using Google's OSS-Fuzz in the libvirt and
> QEMU communities yet.  Maybe it would be a good platform to build
> upon:
yeah, that's a very interesting project. I do not think there has been
serious discussion about it among libvirt and QEMU communities except
some mentioning [1]. I think it could be actually benificial for this
project if at some point we start working on oss-fuzz for libvirt
because they share the fundamental ideas, though the proposal of this
fuzzing project starts from a different perspective.

[1] https://www.redhat.com/archives/libvir-list/2017-May/msg00196.html

Cheers,

Dan
> https://github.com/google/oss-fuzz
> 
> Stefan

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

[libvirt] [PATCH] lxc: allow defining environment variables

[Cédric Bosdonnat]

When running an application container, setting environment variables
could be important.

The newly introduced  tag in domain configuration will allow
setting environment variables to the init program.
---
 docs/formatdomain.html.in|  5 +
 docs/schemas/domaincommon.rng| 10 ++
 src/conf/domain_conf.c   | 38 
 src/conf/domain_conf.h   |  8 
 src/lxc/lxc_container.c  |  5 +
 tests/lxcxml2xmldata/lxc-initenv.xml | 30 
 tests/lxcxml2xmltest.c   |  1 +
 7 files changed, 97 insertions(+)
 create mode 100644 tests/lxcxml2xmldata/lxc-initenv.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 07208eef8..8da50875b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -326,6 +326,10 @@
   element, if set will be used to provide an equivalent to 
/proc/cmdline
   but will not affect init argv.
 
+
+  To set environment variables, use the initenv element, one
+  for each variable.
+
 
 
 os
@@ -333,6 +337,7 @@
   init/bin/systemd/init
   initarg--unit/initarg
   initargemergency.service/initarg
+  initenv name='MYENV'some value/initenv
 /os
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4d9f8d1a2..695214816 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -385,6 +385,16 @@
 
   
 
+
+  
+
+  
+[a-zA-Z_]+[a-zA-Z0-9_]*
+  
+
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c7e20b8ba..89c803047 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2799,6 +2799,9 @@ void virDomainDefFree(virDomainDefPtr def)
 for (i = 0; def->os.initargv && def->os.initargv[i]; i++)
 VIR_FREE(def->os.initargv[i]);
 VIR_FREE(def->os.initargv);
+for (i = 0; def->os.initenv && def->os.initenv[i]; i++)
+VIR_FREE(def->os.initenv[i]);
+VIR_FREE(def->os.initenv);
 VIR_FREE(def->os.kernel);
 VIR_FREE(def->os.initrd);
 VIR_FREE(def->os.cmdline);
@@ -16776,6 +16779,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def,
 xmlNodePtr *nodes = NULL;
 xmlNodePtr oldnode;
 char *tmp = NULL;
+char *name = NULL;
 int ret = -1;
 size_t i;
 int n;
@@ -16811,6 +16815,37 @@ virDomainDefParseBootOptions(virDomainDefPtr def,
 }
 def->os.initargv[n] = NULL;
 VIR_FREE(nodes);
+
+if ((n = virXPathNodeSet("./os/initenv", ctxt, )) < 0)
+goto error;
+
+if (VIR_ALLOC_N(def->os.initenv, n+1) < 0)
+goto error;
+for (i = 0; i < n; i++) {
+if (!(name = virXMLPropString(nodes[i], "name"))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+_("No name supplied for  element"));
+goto error;
+}
+
+if (!nodes[i]->children ||
+!nodes[i]->children->content) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("No value supplied for  
element"),
+   name);
+goto error;
+}
+
+if (VIR_ALLOC(def->os.initenv[i]) < 0)
+goto error;
+
+def->os.initenv[i]->name = name;
+if (VIR_STRDUP(def->os.initenv[i]->value,
+   (const char*) nodes[i]->children->content) < 0)
+goto error;
+}
+def->os.initenv[n] = NULL;
+VIR_FREE(nodes);
 }
 
 if (def->os.type == VIR_DOMAIN_OSTYPE_XEN ||
@@ -24524,6 +24559,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 for (i = 0; def->os.initargv && def->os.initargv[i]; i++)
 virBufferEscapeString(buf, "%s\n",
   def->os.initargv[i]);
+for (i = 0; def->os.initenv && def->os.initenv[i]; i++)
+virBufferAsprintf(buf, "%s\n",
+  def->os.initenv[i]->name, def->os.initenv[i]->value);
 if (def->os.loader)
 virDomainLoaderDefFormat(buf, def->os.loader);
 virBufferEscapeString(buf, "%s\n",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 83e067269..03153b972 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1820,6 +1820,13 @@ typedef enum {
 VIR_ENUM_DECL(virDomainIOAPIC);
 
 /* Operating system configuration data & machine / arch */
+typedef struct _virDomainOSEnv virDomainOSEnv;
+typedef virDomainOSEnv *virDomainOSEnvPtr;
+struct _virDomainOSEnv {
+char *name;
+char *value;
+};
+
 typedef struct _virDomainOSDef virDomainOSDef;
 typedef virDomainOSDef *virDomainOSDefPtr;
 struct _virDomainOSDef {
@@ -1833,6 +1840,7 @@ struct _virDomainOSDef {
 bool bm_timeout_set;
 char 

Re: [libvirt] [PATCH] CI: also run tests using updated distro(s)

[Claudio André]

Em 29/05/2017 08:54, Martin Kletzander escreveu:
On Sun, May 28, 2017 at 11:07:41PM -0300, claudioandre...@gmail.com 
wrote:

From: Claudio André 


'modprobe -c' fails for some reason. 


Ahh! Well, it is possible to run modprobe inside Docker if I enable some 
options. I'll

check it.

Is the user inside the container root?  I don't think so.  Some more 
info output

could be nice, like 'uname -a', 'id', and so on.


I did it.


One disadvantage for using the script is that it couples al output
together.  I know it's needed for running that inside the container, but
the installation and configuration parts take lot of output that I had
to scroll through.  Could _that_ be separated at least?


You can fold/collapsed the output yourself. That said, I folded the 
autogen part, but I'll

keep the make unfolded (the way Travis does). Please, let me know if you
want anything else folded (e.g., hide everything but tests).

And, please:
- access https://travis-ci.org/claudioandre/libvirt/jobs/237625211
- and look at line 485 to see the folding in action


Also in your config (not here), you have some differences, for example
compiler: ["gcc"] even though it doesn't take arrays.  It doesn't
matter, I know, it just confused me a bit :D


This was Travis itself. I fixed it, please, confirm that using the link 
seen above.



Somehow the -qq didn't work, or you didn't have it in the script in the
build you sent the link to.  That's ione of the things hidden fromt he
Web UI.


Without -qq is much worse. We can:
1. collapse as seen above;
2. redirect stdout to /dev/null (I mean, there is no important 
information here).


I prefer (and use) #2.

The silent_if_passes() has some drawbacks.

Claudio

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

Re: [libvirt] [PATCH] maint: add sanitizers to the build process

[Daniel P. Berrange]

On Tue, May 30, 2017 at 12:28:57PM -0300, claudioandre...@gmail.com wrote:
> From: Claudio André 
> 
> Sanitizers are based on compile-time instrumentation. They are available in 
> gcc and clang for a range of supported operation systems and platforms. More 
> info at: https://github.com/google/sanitizers

Please line wrap your commit messages at 80 chars or less.


> diff --git a/m4/virt-compile-sanitizer.m4 b/m4/virt-compile-sanitizer.m4
> new file mode 100644
> index 000..a7cac31
> --- /dev/null
> +++ b/m4/virt-compile-sanitizer.m4
> @@ -0,0 +1,51 @@
> +dnl
> +dnl Check for support for Sanitizers
> +dnl Check for -fsanitize=address and -fsanitize=undefined support
> +dnl
> +dnl This library is free software; you can redistribute it and/or
> +dnl modify it under the terms of the GNU Lesser General Public
> +dnl License as published by the Free Software Foundation; either
> +dnl version 2.1 of the License, or (at your option) any later version.
> +dnl
> +dnl This library is distributed in the hope that it will be useful,
> +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
> +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +dnl Lesser General Public License for more details.
> +dnl
> +dnl You should have received a copy of the GNU Lesser General Public
> +dnl License along with this library.  If not, see
> +dnl .
> +dnl
> +
> +AC_DEFUN([LIBVIRT_COMPILE_SANITIZER],[
> +LIBVIRT_ARG_ENABLE([ASAN], [Build with address sanitizer support], [no])
> +LIBVIRT_ARG_ENABLE([UBSAN], [Build with undefined behavior sanitizer 
> support], [no])
> +
> +SAN_CFLAGS=
> +SAN_LDFLAGS=
> +
> +AS_IF([test "x$enable_asan" = "xyes"], [
> +gl_COMPILER_OPTION_IF([-fsanitize=address -fno-omit-frame-pointer], [
> +SAN_CFLAGS="-fsanitize=address -fno-omit-frame-pointer"
> +SAN_LDFLAGS="-fsanitize=address"
> +])
> +
> +AC_SUBST([SAN_CFLAGS])
> +AC_SUBST([SAN_LDFLAGS])
> +])
> +
> +AS_IF([test "x$enable_ubsan" = "xyes"], [
> +gl_COMPILER_OPTION_IF([-fsanitize=undefined 
> -fno-omit-frame-pointer], [
> +SAN_CFLAGS="$SAN_CFLAGS -fsanitize=undefined 
> -fno-omit-frame-pointer"
> +SAN_LDFLAGS="$SAN_LDFLAGS -fsanitize=undefined"
> +])
> +
> +AC_SUBST([SAN_CFLAGS])
> +AC_SUBST([SAN_LDFLAGS])
> +])
> +])
> +
> +AC_DEFUN([LIBVIRT_RESULT_SANITIZER], [
> +  AC_MSG_NOTICE([  ASan: $enable_asan])
> +  AC_MSG_NOTICE([ UBSan: $enable_ubsan])
> +])

IMHO this could just be added to m4/virt-compile-warnings.m4, rather than
needing to define new CFLAGS/LDFLAGFS variables.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] maint: add sanitizers to the build process

[Claudio André]

Sorry, I mean RFC.

Em 30/05/2017 12:28, claudioandre...@gmail.com escreveu:

From: Claudio André 

Sanitizers are based on compile-time instrumentation. They are available in gcc 
and clang for a range of supported operation systems and platforms. More info 
at: https://github.com/google/sanitizers


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

[libvirt] [PATCH] maint: add sanitizers to the build process

[claudioandre . br]

From: Claudio André 

Sanitizers are based on compile-time instrumentation. They are available in gcc 
and clang for a range of supported operation systems and platforms. More info 
at: https://github.com/google/sanitizers

The address sanitizer finds bugs related to addressing memory: use after free, 
heap buffer overflow, stack buffer overflow, memory leaks, ...
The undefined behavior sanitizer detects situations not prescribed by the 
language specification: bound violations, data overflows, ...

The llvm.org states that Sanitizers have found thousands of bugs everywhere.
Sanitizers running during CI can prevent bugs from taking up residence. A 
helper tool to keep bugs out.
---
- I mean CI (in general) not only Travis;
- The functionality is not tied to CI; it is useful for local testing;
- A way to think about this (including the ongoing GSOC):
  - Phase 1: test with Sanitizers to achieve basic code sanity;
  - Phase 2: use fuzzing for stronger security & reliability;
- MISSING: should I add the flag to which Makefile.am? Or, what do you guys 
think about this?

 configure.ac |  2 ++
 m4/virt-compile-sanitizer.m4 | 51 
 2 files changed, 53 insertions(+)
 create mode 100644 m4/virt-compile-sanitizer.m4

diff --git a/configure.ac b/configure.ac
index 246f4e0..4334614 100644
--- a/configure.ac
+++ b/configure.ac
@@ -237,6 +237,7 @@ LIBVIRT_COMPILE_WARNINGS
 LIBVIRT_COMPILE_PIE
 LIBVIRT_LINKER_RELRO
 LIBVIRT_LINKER_NO_INDIRECT
+LIBVIRT_COMPILE_SANITIZER
 
 LIBVIRT_ARG_APPARMOR
 LIBVIRT_ARG_ATTR
@@ -1011,6 +1012,7 @@ AC_MSG_NOTICE([])
 AC_MSG_NOTICE([Miscellaneous])
 AC_MSG_NOTICE([])
 LIBVIRT_RESULT_DEBUG
+LIBVIRT_RESULT_SANITIZER
 AC_MSG_NOTICE([   Use -Werror: $enable_werror])
 AC_MSG_NOTICE([ Warning Flags: $WARN_CFLAGS])
 LIBVIRT_RESULT_DTRACE
diff --git a/m4/virt-compile-sanitizer.m4 b/m4/virt-compile-sanitizer.m4
new file mode 100644
index 000..a7cac31
--- /dev/null
+++ b/m4/virt-compile-sanitizer.m4
@@ -0,0 +1,51 @@
+dnl
+dnl Check for support for Sanitizers
+dnl Check for -fsanitize=address and -fsanitize=undefined support
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl .
+dnl
+
+AC_DEFUN([LIBVIRT_COMPILE_SANITIZER],[
+LIBVIRT_ARG_ENABLE([ASAN], [Build with address sanitizer support], [no])
+LIBVIRT_ARG_ENABLE([UBSAN], [Build with undefined behavior sanitizer 
support], [no])
+
+SAN_CFLAGS=
+SAN_LDFLAGS=
+
+AS_IF([test "x$enable_asan" = "xyes"], [
+gl_COMPILER_OPTION_IF([-fsanitize=address -fno-omit-frame-pointer], [
+SAN_CFLAGS="-fsanitize=address -fno-omit-frame-pointer"
+SAN_LDFLAGS="-fsanitize=address"
+])
+
+AC_SUBST([SAN_CFLAGS])
+AC_SUBST([SAN_LDFLAGS])
+])
+
+AS_IF([test "x$enable_ubsan" = "xyes"], [
+gl_COMPILER_OPTION_IF([-fsanitize=undefined -fno-omit-frame-pointer], [
+SAN_CFLAGS="$SAN_CFLAGS -fsanitize=undefined 
-fno-omit-frame-pointer"
+SAN_LDFLAGS="$SAN_LDFLAGS -fsanitize=undefined"
+])
+
+AC_SUBST([SAN_CFLAGS])
+AC_SUBST([SAN_LDFLAGS])
+])
+])
+
+AC_DEFUN([LIBVIRT_RESULT_SANITIZER], [
+  AC_MSG_NOTICE([  ASan: $enable_asan])
+  AC_MSG_NOTICE([ UBSan: $enable_ubsan])
+])
-- 
2.11.0

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

Re: [libvirt] [PATCH 5/9] util: Introduce virObjectLockableRecursiveNew

[John Ferlan]

On 05/30/2017 09:19 AM, Daniel P. Berrange wrote:
> On Tue, May 30, 2017 at 03:03:14PM +0200, Pavel Hrdina wrote:
>> On Tue, May 30, 2017 at 07:38:17AM -0400, John Ferlan wrote:
>>> Introduce a recursive option for the lockable object which calls
>>> virMutexInitRecursive instead of virMutexInit.
>>
>> We should avoid using recursive lock because they are dangerous.  I
>> would rather cleanup the nwfilter code than introducing more stuff for
>> recursive locks.
> 
> Last time I looked I came to the conclusion that it wasn't possible to
> remove the recursive locking from nwfilter.
> 
> I agree about /not/ introducing new usage of recursive locking though.
> 

So this is a chicken/egg problem Cannot create a new API to use
recursive locks and  cannot remove the recursive lock without having
adjustments to nwfilter code that would seemingly require self locking
list objects to get hash table elements.

If the comments in src/nwfilter/nwfilter_gentech_driver.c (search on
XXX) could prove to be true, then it would be possible to not have a
recursive lock. Perhaps that loop could be rewritten in virnwfilterobj
to grab a reference to a hash table object while releasing the object
lock that's causing the deadlock.  I haven't yet circled back to
determine whether this is possible yet, but since code in my branches is
at least this far, I can look again.

John

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

Re: [libvirt] [PATCH 4/9] util: Add magic_marker for all virObjects

[Daniel P. Berrange]

On Tue, May 30, 2017 at 10:43:35AM -0400, John Ferlan wrote:
> 
> 
> On 05/30/2017 09:00 AM, Daniel P. Berrange wrote:
> > On Tue, May 30, 2017 at 07:38:16AM -0400, John Ferlan wrote:
> >> The virObject logic "assumes" that whatever is passed to its API's
> >> would be some sort of virObjectPtr; however, if it is not then some
> >> really bad things can happen.
> >>
> >> So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock},
> >> and virObjectIsClass and the virObject and virObjectLockable class
> >> consumers have been well behaved and code well tested. Soon there will
> >> be more consumers and one such consumer tripped over this during testing
> >> by passing a virHashTablePtr to virObjectIsClass which ends up calling
> >> virClassIsDerivedFrom using "obj->klass", which wasn't really a klass
> >> object causing one of those bad things to happen.
> >>
> >> To avoid the future possibility that a non virObject class memory was
> >> passed to some virObject* API, let's add a "magic_marker" to the base
> >> virObject class that contains a value "0xFEEDBEEF", then any place in
> >> the code which would accept or process the base opaque virObjectPtr,
> >> compare the magic_marker against 0xFEEDBEEF to make sure this is an
> >> object allocated by this code.
> >>
> >> It is still left up to the caller to handle the failed API calls just
> >> as it would be if it passed a NULL opaque pointer anyobj.
> >>
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  src/util/virobject.c | 12 
> >>  src/util/virobject.h |  1 +
> >>  2 files changed, 9 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/util/virobject.c b/src/util/virobject.c
> >> index 9f5f187..a1934941 100644
> >> --- a/src/util/virobject.c
> >> +++ b/src/util/virobject.c
> >> @@ -47,10 +47,12 @@ struct _virClass {
> >>  virObjectDisposeCallback dispose;
> >>  };
> >>  
> >> +#define VIR_OBJECT_NOTVALID(obj) (!obj || obj->magic_marker != 0xFEEDBEEF)
> >> +
> >>  #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass)  
> >>   \
> >>  do {  
> >>   \
> >>  virObjectPtr obj = anyobj;
> >>   \
> >> -if (!obj) 
> >>   \
> >> +if (VIR_OBJECT_NOTVALID(obj)) 
> >>   \
> >>  VIR_WARN("Object %p is not a virObject class instance", 
> >> anyobj);\
> >>  else  
> >>   \
> >>  VIR_WARN("Object %p (%s) is not a %s instance",   
> >>   \
> >> @@ -212,6 +214,7 @@ virObjectNew(virClassPtr klass)
> >>  return NULL;
> >>  
> >>  obj->u.s.magic = klass->magic;
> >> +obj->magic_marker = 0xFEEDBEEF;
> >>  obj->klass = klass;
> >>  virAtomicIntSet(>u.s.refs, 1);
> >>  
> >> @@ -272,7 +275,7 @@ virObjectUnref(void *anyobj)
> >>  {
> >>  virObjectPtr obj = anyobj;
> >>  
> >> -if (!obj)
> >> +if (VIR_OBJECT_NOTVALID(obj))
> >>  return false;
> >>  
> >>  bool lastRef = virAtomicIntDecAndTest(>u.s.refs);
> >> @@ -289,6 +292,7 @@ virObjectUnref(void *anyobj)
> >>  /* Clear & poison object */
> >>  memset(obj, 0, obj->klass->objectSize);
> >>  obj->u.s.magic = 0xDEADBEEF;
> >> +obj->magic_marker = 0xDEADBEEF;
> >>  obj->klass = (void*)0xDEADBEEF;
> >>  VIR_FREE(obj);
> >>  }
> >> @@ -311,7 +315,7 @@ virObjectRef(void *anyobj)
> >>  {
> >>  virObjectPtr obj = anyobj;
> >>  
> >> -if (!obj)
> >> +if (VIR_OBJECT_NOTVALID(obj))
> >>  return NULL;
> >>  virAtomicIntInc(>u.s.refs);
> >>  PROBE(OBJECT_REF, "obj=%p", obj);
> >> @@ -389,7 +393,7 @@ virObjectIsClass(void *anyobj,
> >>   virClassPtr klass)
> >>  {
> >>  virObjectPtr obj = anyobj;
> >> -if (!obj)
> >> +if (VIR_OBJECT_NOTVALID(obj))
> >>  return false;
> >>  
> >>  return virClassIsDerivedFrom(obj->klass, klass);
> >> diff --git a/src/util/virobject.h b/src/util/virobject.h
> >> index f4c292b..89f8050 100644
> >> --- a/src/util/virobject.h
> >> +++ b/src/util/virobject.h
> >> @@ -51,6 +51,7 @@ struct _virObject {
> >>  int refs;
> >>  } s;
> >>  } u;
> >> +unsigned int magic_marker;
> >>  virClassPtr klass;
> >>  };
> > 
> > I'm wondering whether this will risk re-introducing the bug fixed in
> > 
> >   commit fca4f2334072d87f7faeb2948e6f83201309e1b9
> >   Author: Eric Blake 
> >   Date:   Thu Dec 12 16:01:15 2013 -0700
> > 
> > object: require maximal alignment in base class
> > 
> > I'm also thinking we don't really need to have 2 magic fields in the
> > same struct - we already have a 'magic' field that is initialized from
> > the class object magic value.
> > 
> > Now, this existing magic is different for each 

Re: [libvirt] [PATCH 4/9] util: Add magic_marker for all virObjects

[John Ferlan]

On 05/30/2017 09:00 AM, Daniel P. Berrange wrote:
> On Tue, May 30, 2017 at 07:38:16AM -0400, John Ferlan wrote:
>> The virObject logic "assumes" that whatever is passed to its API's
>> would be some sort of virObjectPtr; however, if it is not then some
>> really bad things can happen.
>>
>> So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock},
>> and virObjectIsClass and the virObject and virObjectLockable class
>> consumers have been well behaved and code well tested. Soon there will
>> be more consumers and one such consumer tripped over this during testing
>> by passing a virHashTablePtr to virObjectIsClass which ends up calling
>> virClassIsDerivedFrom using "obj->klass", which wasn't really a klass
>> object causing one of those bad things to happen.
>>
>> To avoid the future possibility that a non virObject class memory was
>> passed to some virObject* API, let's add a "magic_marker" to the base
>> virObject class that contains a value "0xFEEDBEEF", then any place in
>> the code which would accept or process the base opaque virObjectPtr,
>> compare the magic_marker against 0xFEEDBEEF to make sure this is an
>> object allocated by this code.
>>
>> It is still left up to the caller to handle the failed API calls just
>> as it would be if it passed a NULL opaque pointer anyobj.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/util/virobject.c | 12 
>>  src/util/virobject.h |  1 +
>>  2 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/util/virobject.c b/src/util/virobject.c
>> index 9f5f187..a1934941 100644
>> --- a/src/util/virobject.c
>> +++ b/src/util/virobject.c
>> @@ -47,10 +47,12 @@ struct _virClass {
>>  virObjectDisposeCallback dispose;
>>  };
>>  
>> +#define VIR_OBJECT_NOTVALID(obj) (!obj || obj->magic_marker != 0xFEEDBEEF)
>> +
>>  #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass)
>> \
>>  do {
>> \
>>  virObjectPtr obj = anyobj;  
>> \
>> -if (!obj)   
>> \
>> +if (VIR_OBJECT_NOTVALID(obj))   
>> \
>>  VIR_WARN("Object %p is not a virObject class instance", 
>> anyobj);\
>>  else
>> \
>>  VIR_WARN("Object %p (%s) is not a %s instance", 
>> \
>> @@ -212,6 +214,7 @@ virObjectNew(virClassPtr klass)
>>  return NULL;
>>  
>>  obj->u.s.magic = klass->magic;
>> +obj->magic_marker = 0xFEEDBEEF;
>>  obj->klass = klass;
>>  virAtomicIntSet(>u.s.refs, 1);
>>  
>> @@ -272,7 +275,7 @@ virObjectUnref(void *anyobj)
>>  {
>>  virObjectPtr obj = anyobj;
>>  
>> -if (!obj)
>> +if (VIR_OBJECT_NOTVALID(obj))
>>  return false;
>>  
>>  bool lastRef = virAtomicIntDecAndTest(>u.s.refs);
>> @@ -289,6 +292,7 @@ virObjectUnref(void *anyobj)
>>  /* Clear & poison object */
>>  memset(obj, 0, obj->klass->objectSize);
>>  obj->u.s.magic = 0xDEADBEEF;
>> +obj->magic_marker = 0xDEADBEEF;
>>  obj->klass = (void*)0xDEADBEEF;
>>  VIR_FREE(obj);
>>  }
>> @@ -311,7 +315,7 @@ virObjectRef(void *anyobj)
>>  {
>>  virObjectPtr obj = anyobj;
>>  
>> -if (!obj)
>> +if (VIR_OBJECT_NOTVALID(obj))
>>  return NULL;
>>  virAtomicIntInc(>u.s.refs);
>>  PROBE(OBJECT_REF, "obj=%p", obj);
>> @@ -389,7 +393,7 @@ virObjectIsClass(void *anyobj,
>>   virClassPtr klass)
>>  {
>>  virObjectPtr obj = anyobj;
>> -if (!obj)
>> +if (VIR_OBJECT_NOTVALID(obj))
>>  return false;
>>  
>>  return virClassIsDerivedFrom(obj->klass, klass);
>> diff --git a/src/util/virobject.h b/src/util/virobject.h
>> index f4c292b..89f8050 100644
>> --- a/src/util/virobject.h
>> +++ b/src/util/virobject.h
>> @@ -51,6 +51,7 @@ struct _virObject {
>>  int refs;
>>  } s;
>>  } u;
>> +unsigned int magic_marker;
>>  virClassPtr klass;
>>  };
> 
> I'm wondering whether this will risk re-introducing the bug fixed in
> 
>   commit fca4f2334072d87f7faeb2948e6f83201309e1b9
>   Author: Eric Blake 
>   Date:   Thu Dec 12 16:01:15 2013 -0700
> 
> object: require maximal alignment in base class
> 
> I'm also thinking we don't really need to have 2 magic fields in the
> same struct - we already have a 'magic' field that is initialized from
> the class object magic value.
> 
> Now, this existing magic is different for each object subclass - we allocate
> class magic starting with
> 
>   static unsigned int magicCounter = 0xCAFE;
> 
> I'm thinking though, that we're never going to have > 65556 different
> sub-classes (well at least not for a long time).
> 
> So instead of adding this new field you could just check
> 
>  

Re: [libvirt] [PATCH 4/9] util: Add magic_marker for all virObjects

[John Ferlan]

On 05/30/2017 08:50 AM, Pavel Hrdina wrote:
> On Tue, May 30, 2017 at 07:38:16AM -0400, John Ferlan wrote:
>> The virObject logic "assumes" that whatever is passed to its API's
>> would be some sort of virObjectPtr; however, if it is not then some
>> really bad things can happen.
>>
>> So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock},
>> and virObjectIsClass and the virObject and virObjectLockable class
>> consumers have been well behaved and code well tested. Soon there will
>> be more consumers and one such consumer tripped over this during testing
>> by passing a virHashTablePtr to virObjectIsClass which ends up calling
>> virClassIsDerivedFrom using "obj->klass", which wasn't really a klass
>> object causing one of those bad things to happen.
>>
>> To avoid the future possibility that a non virObject class memory was
>> passed to some virObject* API, let's add a "magic_marker" to the base
>> virObject class that contains a value "0xFEEDBEEF", then any place in
>> the code which would accept or process the base opaque virObjectPtr,
>> compare the magic_marker against 0xFEEDBEEF to make sure this is an
>> object allocated by this code.
>>
>> It is still left up to the caller to handle the failed API calls just
>> as it would be if it passed a NULL opaque pointer anyobj.
> 
> I don't think that we need to guard this kind of programming error.
> We've been able to cope with the current virObject code without hitting
> this issue.
> 

I don't disagree that we've been able to cope, but adding more objects
makes things more interesting and prone to having code trip across a bad
usage which we could have prevented. The base object classes make a lot
of assumptions and use opaque params, so having some sort of check
prevents those possibly very bad things happening. Not everyone is well
behaved and at times we do make assumptions. IIRC it took me a bit of
combing through what I'd changed to figure out my erroneous usage which
resulted in me adding this to make it very much so more obvious.

John

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

Re: [libvirt] [PATCH go-xml] Add support for Node Device with basic testing.

[Vladik Romanovsky]

Thanks for the review.

On Tue, May 30, 2017 at 8:46 AM, Daniel P. Berrange  wrote:
> On Mon, May 29, 2017 at 03:58:52AM -0400, Vladik Romanovsky wrote:
>> ---
>>  node_device.go  | 277 
>> 
>>  node_device_test.go | 111 +
>>  2 files changed, 388 insertions(+)
>>  create mode 100644 node_device.go
>>  create mode 100644 node_device_test.go
>>
>> diff --git a/node_device.go b/node_device.go
>> new file mode 100644
>> index 000..5375d32
>> --- /dev/null
>> +++ b/node_device.go
>> @@ -0,0 +1,277 @@
>> +/*
>> + * This file is part of the libvirt-go-xml project
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a 
>> copy
>> + * of this software and associated documentation files (the "Software"), to 
>> deal
>> + * in the Software without restriction, including without limitation the 
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included 
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
>> THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + *
>> + * Copyright (C) 2017 Red Hat, Inc.
>> + *
>> + */
>> +
>> +package libvirtxml
>> +
>> +import (
>> + "encoding/xml"
>> + "fmt"
>> +)
>> +
>> +type NodeDevice struct {
>> + Name   string  `xml:"name"`
>> + Path   string  `xml:"path,omitempty"`
>> + Parent string  `xml:"parent,omitempty"`
>> + Driver string  `xml:"driver>name,omitempty"`
>> + Capability *CapabilityType `xml:"capability"`
>> +}
>
> A single device can have multiple capabilities.
Do you mean it should be:
 Capability []CapabilityType `xml:"capability"` ?

>
>> +
>> +type CapabilityType struct {
>> + Type interface{} `xml:"type,attr"`
>> +}
>
> I'm not convinced about this approach - it differs from what we've
> done in other schemas, where we just have a single struct containing
> the union of data from all types. The user has no idea what they
> should be providing for 'Type' here since its a generic interface
> type.
>
Yes. I wasn't sure about this either. Originally, I wanted to avoid
the Type attribute and make
CapabilityType to be an interface type.
However, I figured that it's not possible to write an UnmarshalXML for
an interface type.

The reason I didn't use the same approach as in the rest of the
schemas is because the Capability
determins it's type by a XML attribute and not an element name.
I didn't find an easy way to query the xml attribue as something
like.. `xml:"type=something,attr"`

OK, I'll try a ddiffernt approach.

>>
>> +type IDName struct {
>> + ID   string `xml:"id,attr"`
>> + Name string `xml:",chardata"`
>> +}
>> +
>> +type PciExpress struct {
>> + Links []PciExpressLink `xml:"link"`
>> +}
>> +
>> +type PciExpressLink struct {
>> + Validity string  `xml:"validity,attr,omitempty"`
>> + Speedfloat64 `xml:"speed,attr,omitempty"`
>> + Port int `xml:"port,attr,omitempty"`
>> + Widthint `xml:"width,attr,omitempty"`
>> +}
>> +
>> +type IOMMUGroupType struct {
>> + Number int `xml:"number,attr"`
>> +}
>> +
>> +type NUMA struct {
>> + Node int `xml:"node,attr"`
>> +}
>> +
>> +type PciCapabilityType struct {
>> + Domain   int `xml:"domain,omitempty"`
>> + Bus  int `xml:"bus,omitempty"`
>> + Slot int `xml:"slot,omitempty"`
>> + Function int `xml:"function,omitempty"`
>> + Product  IDName  `xml:"product,omitempty"`
>> + Vendor   IDName  `xml:"vendor,omitempty"`
>> + IommuGroup   IOMMUGroupType  `xml:"iommuGroup,omitempty"`
>> + Numa NUMA`xml:"numa,omitempty"`
>> + PciExpress   PciExpress  `xml:"pci-express,omitempty"`
>> + Capabilities []PciCapability `xml:"capability,omitempty"`
>> +}
>> +
>> +type PCIAddress struct {
>> + Domain   string `xml:"domain,attr"`
>> + Bus  string `xml:"bus,attr"`
>> + Slot string `xml:"slot,attr"`
>> + Function string `xml:"function,attr"`
>> +}
>> +
>> +type PciCapability struct {
>
> There's alot of inconsistency in capitalization of abbreviations
> in 

Re: [libvirt] [PATCH v3 2/3] qemu : Add loadparm to qemu command line string

[Farhan Ali]

On 05/26/2017 04:03 PM, John Ferlan wrote:



...


Was there ever thought to adding loadparm to the machine XML? What's the
reasoning to not have it there.  If it's only valid for bootindex=1,
then it's far easier to check if the machine XML has it defined rather
than perusing the disk/network lists (which could be lengthy) only to
fine none.  If the concern is some other arch allowing a different
bootindex to have loadparm, then the simple decision there is to have a
"loadparm_bootindex=#" value that would match the disk bootindex=# value.



I am not sure what you mean here? By machine xml do you mean  xml?



Sorry I was too lazy to go make the cross reference near the end of the
day/review. Guess I was thinking more  related though...

I see in  there's a  subelement which has a relationship with
the  subelement for ...[|]...



Yes, the  has  subelement, but it can be tricky to specify the 
order of the boot device.



I think I was just trying to make sure that adding  to
 would make sense "in the long run" and what other
implementations were considered and not taken because of some drawback.



The per device boot subelement was added to simplify the boot device 
order. That's why I thought it made more sense to add it to the per 
device boot element. Also from a user's perspective it's easier to 
specify the loadparm when specifying the boot order.



Given the description I've read and the implementation that searches
either disk or network lists looking for bootIndex = 1, I wonder if the
  should be modified instead.  Was that considered
- what were the drawbacks?   Can bootparm conceptually be added/used for
a non boot disk?




I'm not requesting one way or another - I'd just like to be sure the
question is answered before perhaps someone else asks it now or much
later when this isn't so fresh in your mind.

John




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

Re: [libvirt] [PATCH v3 9/9] interface: Convert virInterfaceObj to use virObjectLockable

[John Ferlan]

On 05/30/2017 08:13 AM, Bjoern Walk wrote:
> John Ferlan  [2017-05-30, 06:43AM -0400]:
>> [...]
>> void
>> -virInterfaceObjFree(virInterfaceObjPtr obj)
>> +virInterfaceObjEndAPI(virInterfaceObjPtr *obj)
> 
> Naming is hard, and I don't have any better suggestion. Just wanted to
> say that the name is, maybe, improvable :)
> 

It's a common nomenclature for libvirt... virDomainObjEndAPI,
virNetworkObjEndAPI, and virSecretObjEndAPI.

>> {
>> -if (!obj)
>> +if (!*obj)
>> return;
>>
>> -virInterfaceDefFree(obj->def);
>> -virMutexDestroy(>lock);
>> -VIR_FREE(obj);
>> +virObjectUnlock(*obj);
>> +virObjectUnref(*obj);
>> +*obj = NULL;
> 
> I'm a bit conflicted if this is strictly necessary. In what situation
> would this bite a consumer if we don't NULL obj? At least in this patch
> I can't find any.
> 

Although perhaps true - it's the common way this happens for other
vir*ObjEndAPI source code. Since it's theoretically possible an Unref
could cause the object to be Disposed (if refcnt == 0), setting *obj =
NULL at least "ensures" the caller misusing the object would be a NULL
deref rather than referencing something it no longer owned.

>> }
>>
>>
>> @@ -140,18 +150,18 @@
>> virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
>> virInterfaceObjPtr obj = interfaces->objs[i];
>> virInterfaceDefPtr def;
>>
>> -virInterfaceObjLock(obj);
>> +virObjectLock(obj);
>> def = obj->def;
>> if (STRCASEEQ(def->mac, mac)) {
>> if (matchct < maxmatches) {
>> if (VIR_STRDUP(matches[matchct], def->name) < 0) {
>> -virInterfaceObjUnlock(obj);
>> +virObjectUnlock(obj);
>> goto error;
>> }
>> matchct++;
>> }
>> }
>> -virInterfaceObjUnlock(obj);
>> +virObjectUnlock(obj);
>> }
>> return matchct;
>>
>> @@ -173,11 +183,11 @@
>> virInterfaceObjListFindByName(virInterfaceObjListPtr interfaces,
>> virInterfaceObjPtr obj = interfaces->objs[i];
>> virInterfaceDefPtr def;
>>
>> -virInterfaceObjLock(obj);
>> +virObjectLock(obj);
>> def = obj->def;
>> if (STREQ(def->name, name))
>> -return obj;
>> -virInterfaceObjUnlock(obj);
>> +return virObjectRef(obj);
>> +virObjectUnlock(obj);
>> }
>>
>> return NULL;
>> @@ -190,7 +200,7 @@ virInterfaceObjListFree(virInterfaceObjListPtr
>> interfaces)
>> size_t i;
>>
>> for (i = 0; i < interfaces->count; i++)
>> -virInterfaceObjFree(interfaces->objs[i]);
>> +virObjectUnref(interfaces->objs[i]);
>> VIR_FREE(interfaces->objs);
>> VIR_FREE(interfaces);
>> }
>> @@ -227,7 +237,7 @@ virInterfaceObjListClone(virInterfaceObjListPtr
>> interfaces)
>> VIR_FREE(xml);
>> if (!(obj = virInterfaceObjListAssignDef(dest, backup)))
>> goto error;
>> -virInterfaceObjUnlock(obj); /* locked by
>> virInterfaceObjListAssignDef */
>> +virInterfaceObjEndAPI();
>> }
>>
>> return dest;
>> @@ -256,13 +266,12 @@
>> virInterfaceObjListAssignDef(virInterfaceObjListPtr interfaces,
>>
>> if (VIR_APPEND_ELEMENT_COPY(interfaces->objs,
>> interfaces->count, obj) < 0) {
>> -virInterfaceObjUnlock(obj);
>> -virInterfaceObjFree(obj);
>> +virInterfaceObjEndAPI();
>> return NULL;
>> }
>>
>> obj->def = def;
>> -return obj;
>> +return virObjectRef(obj);
>>
>> }
>>
>> @@ -273,17 +282,17 @@ virInterfaceObjListRemove(virInterfaceObjListPtr
>> interfaces,
>> {
>> size_t i;
>>
>> -virInterfaceObjUnlock(obj);
>> +virObjectUnlock(obj);
>> for (i = 0; i < interfaces->count; i++) {
>> -virInterfaceObjLock(interfaces->objs[i]);
>> +virObjectLock(interfaces->objs[i]);
>> if (interfaces->objs[i] == obj) {
>> -virInterfaceObjUnlock(interfaces->objs[i]);
>> -virInterfaceObjFree(interfaces->objs[i]);
>> +virObjectUnlock(interfaces->objs[i]);
>> +virObjectUnref(interfaces->objs[i]);
> 
> Here you could use virInterfaceObjEndAPI if the nulling would be
> dropped. Small advantage, I know.

Understood, I suppose this could also have taken the
"virInterfaceObjEndAPI();" option...

Still eventually this is changing from a forward linked list removal to
a removal from a hash table.

For the sake of my local branches, I'd prefer to keep as is, although if
there's a strong desire for something different I'm not opposed to
adjusting.

Thanks for taking a look at the last two!

John

>>
>> VIR_DELETE_ELEMENT(interfaces->objs, i, interfaces->count);
>> break;
>> }
>> -virInterfaceObjUnlock(interfaces->objs[i]);
>> +virObjectUnlock(interfaces->objs[i]);
>> }
>> }
>>
>> @@ -297,10 +306,10 @@
>> 

[libvirt] [PATCH 3/9] util: Generate a common internal only error print

[John Ferlan]

If virObjectIsClass fails "internally" to virobject.c, create a macro to
generate the VIR_WARN describing what the problem is. Also improve the
checks and message a bit to indicate which was the failure - whether the
obj was NULL or just not the right class

Signed-off-by: John Ferlan 
---
 src/util/virobject.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index 34805d3..9f5f187 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -47,6 +47,16 @@ struct _virClass {
 virObjectDisposeCallback dispose;
 };
 
+#define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass)\
+do {\
+virObjectPtr obj = anyobj;  \
+if (!obj)   \
+VIR_WARN("Object %p is not a virObject class instance", anyobj);\
+else\
+VIR_WARN("Object %p (%s) is not a %s instance", \
+  anyobj, obj->klass->name, #objclass); \
+} while (0)
+
 static virClassPtr virObjectClass;
 static virClassPtr virObjectLockableClass;
 
@@ -312,14 +322,10 @@ virObjectRef(void *anyobj)
 static virObjectLockablePtr
 virObjectGetLockableObj(void *anyobj)
 {
-virObjectPtr obj;
-
 if (virObjectIsClass(anyobj, virObjectLockableClass))
 return anyobj;
 
-obj = anyobj;
-VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
-  anyobj, obj ? obj->klass->name : "(unknown)");
+VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectLockableClass);
 
 return NULL;
 }
-- 
2.9.4

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

Re: [libvirt] [PATCH 5/9] util: Introduce virObjectLockableRecursiveNew

[Daniel P. Berrange]

On Tue, May 30, 2017 at 03:03:14PM +0200, Pavel Hrdina wrote:
> On Tue, May 30, 2017 at 07:38:17AM -0400, John Ferlan wrote:
> > Introduce a recursive option for the lockable object which calls
> > virMutexInitRecursive instead of virMutexInit.
> 
> We should avoid using recursive lock because they are dangerous.  I
> would rather cleanup the nwfilter code than introducing more stuff for
> recursive locks.

Last time I looked I came to the conclusion that it wasn't possible to
remove the recursive locking from nwfilter.

I agree about /not/ introducing new usage of recursive locking though.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 09/18] Add virtio-related options to interfaces

[Daniel P. Berrange]

On Tue, May 30, 2017 at 02:50:32PM +0200, Ján Tomko wrote:
> 
>   
>   
>   
> 

Feels like we could just use  'iommu=on' as the name.

Also, I'm unclear whether we actually need the 'ats' attribute.

>From the description it sounds like if we have device_iotlb=on
for the IOMMU device, then we should have ats=on too. Is there
a  reason to use ats=off when device_iotlb=on, or vica-verca.
If not, then we should just do the right thing and set ats=on
whenever we see device_iotlb=on.
 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [GSoC] Project of libvirt/qemu fuzzing

[Stefan Hajnoczi]

On Tue, May 30, 2017 at 12:03 PM, Dan  wrote:
> The project of qemu command line fuzzing has been accepted as a GSoC
> project [1] [2]. As a student participating Google Summer of Code
> activity, I am extremely exitited to get started today on May 30th,
> 2017.

Welcome!  Great project idea, I am looking forward to your contributions.

Do you have a particular fuzzer in mind or will you write a custom
fuzzer from scratch?

I'm not aware of anyone using Google's OSS-Fuzz in the libvirt and
QEMU communities yet.  Maybe it would be a good platform to build
upon:
https://github.com/google/oss-fuzz

Stefan

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

Re: [libvirt] [PATCH 5/9] util: Introduce virObjectLockableRecursiveNew

[Pavel Hrdina]

On Tue, May 30, 2017 at 07:38:17AM -0400, John Ferlan wrote:
> Introduce a recursive option for the lockable object which calls
> virMutexInitRecursive instead of virMutexInit.

We should avoid using recursive lock because they are dangerous.  I
would rather cleanup the nwfilter code than introducing more stuff for
recursive locks.

Pavel


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

Re: [libvirt] [PATCH 4/9] util: Add magic_marker for all virObjects

[Daniel P. Berrange]

On Tue, May 30, 2017 at 07:38:16AM -0400, John Ferlan wrote:
> The virObject logic "assumes" that whatever is passed to its API's
> would be some sort of virObjectPtr; however, if it is not then some
> really bad things can happen.
> 
> So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock},
> and virObjectIsClass and the virObject and virObjectLockable class
> consumers have been well behaved and code well tested. Soon there will
> be more consumers and one such consumer tripped over this during testing
> by passing a virHashTablePtr to virObjectIsClass which ends up calling
> virClassIsDerivedFrom using "obj->klass", which wasn't really a klass
> object causing one of those bad things to happen.
> 
> To avoid the future possibility that a non virObject class memory was
> passed to some virObject* API, let's add a "magic_marker" to the base
> virObject class that contains a value "0xFEEDBEEF", then any place in
> the code which would accept or process the base opaque virObjectPtr,
> compare the magic_marker against 0xFEEDBEEF to make sure this is an
> object allocated by this code.
> 
> It is still left up to the caller to handle the failed API calls just
> as it would be if it passed a NULL opaque pointer anyobj.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virobject.c | 12 
>  src/util/virobject.h |  1 +
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index 9f5f187..a1934941 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -47,10 +47,12 @@ struct _virClass {
>  virObjectDisposeCallback dispose;
>  };
>  
> +#define VIR_OBJECT_NOTVALID(obj) (!obj || obj->magic_marker != 0xFEEDBEEF)
> +
>  #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass)\
>  do {\
>  virObjectPtr obj = anyobj;  \
> -if (!obj)   \
> +if (VIR_OBJECT_NOTVALID(obj))   \
>  VIR_WARN("Object %p is not a virObject class instance", anyobj);\
>  else\
>  VIR_WARN("Object %p (%s) is not a %s instance", \
> @@ -212,6 +214,7 @@ virObjectNew(virClassPtr klass)
>  return NULL;
>  
>  obj->u.s.magic = klass->magic;
> +obj->magic_marker = 0xFEEDBEEF;
>  obj->klass = klass;
>  virAtomicIntSet(>u.s.refs, 1);
>  
> @@ -272,7 +275,7 @@ virObjectUnref(void *anyobj)
>  {
>  virObjectPtr obj = anyobj;
>  
> -if (!obj)
> +if (VIR_OBJECT_NOTVALID(obj))
>  return false;
>  
>  bool lastRef = virAtomicIntDecAndTest(>u.s.refs);
> @@ -289,6 +292,7 @@ virObjectUnref(void *anyobj)
>  /* Clear & poison object */
>  memset(obj, 0, obj->klass->objectSize);
>  obj->u.s.magic = 0xDEADBEEF;
> +obj->magic_marker = 0xDEADBEEF;
>  obj->klass = (void*)0xDEADBEEF;
>  VIR_FREE(obj);
>  }
> @@ -311,7 +315,7 @@ virObjectRef(void *anyobj)
>  {
>  virObjectPtr obj = anyobj;
>  
> -if (!obj)
> +if (VIR_OBJECT_NOTVALID(obj))
>  return NULL;
>  virAtomicIntInc(>u.s.refs);
>  PROBE(OBJECT_REF, "obj=%p", obj);
> @@ -389,7 +393,7 @@ virObjectIsClass(void *anyobj,
>   virClassPtr klass)
>  {
>  virObjectPtr obj = anyobj;
> -if (!obj)
> +if (VIR_OBJECT_NOTVALID(obj))
>  return false;
>  
>  return virClassIsDerivedFrom(obj->klass, klass);
> diff --git a/src/util/virobject.h b/src/util/virobject.h
> index f4c292b..89f8050 100644
> --- a/src/util/virobject.h
> +++ b/src/util/virobject.h
> @@ -51,6 +51,7 @@ struct _virObject {
>  int refs;
>  } s;
>  } u;
> +unsigned int magic_marker;
>  virClassPtr klass;
>  };

I'm wondering whether this will risk re-introducing the bug fixed in

  commit fca4f2334072d87f7faeb2948e6f83201309e1b9
  Author: Eric Blake 
  Date:   Thu Dec 12 16:01:15 2013 -0700

object: require maximal alignment in base class

I'm also thinking we don't really need to have 2 magic fields in the
same struct - we already have a 'magic' field that is initialized from
the class object magic value.

Now, this existing magic is different for each object subclass - we allocate
class magic starting with

  static unsigned int magicCounter = 0xCAFE;

I'm thinking though, that we're never going to have > 65556 different
sub-classes (well at least not for a long time).

So instead of adding this new field you could just check

 ((object->u.s.magic & 0xCAFE) == 0xCAFE)


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: 

[libvirt] [PATCH 18/18] qemu: format virtio-related options on the command line

[Ján Tomko]

Format iommu_platform= and ats= for virtio devices.

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 src/qemu/qemu_capabilities.c   | 12 -
 src/qemu/qemu_capabilities.h   |  2 +
 src/qemu/qemu_command.c| 58 ++
 tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |  2 +
 .../qemuxml2argv-virtio-options.args   | 44 +---
 tests/qemuxml2argvtest.c   |  4 +-
 6 files changed, 102 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ae466d7..296a0c4 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -374,6 +374,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "intel-iommu.eim",
 
   "intel-iommu.device-iotlb", /* 260 */
+  "virtio.iommu_platform",
+  "virtio.ats",
 );
 
 
@@ -1853,7 +1855,7 @@ struct virQEMUCapsPropTypeObjects {
 const char **objects;
 };
 
-static const char *virQEMUCapsVirtioPCIDisableLegacyObjects[] = {
+static const char *virQEMUCapsVirtioPCIObjects[] = {
  "virtio-balloon-pci",
  "virtio-blk-pci",
  "virtio-scsi-pci",
@@ -1872,7 +1874,13 @@ static const char 
*virQEMUCapsVirtioPCIDisableLegacyObjects[] = {
 static struct virQEMUCapsPropTypeObjects virQEMUCapsPropObjects[] = {
 { "disable-legacy",
   QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
-  virQEMUCapsVirtioPCIDisableLegacyObjects }
+  virQEMUCapsVirtioPCIObjects },
+{ "iommu_platform",
+  QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM,
+  virQEMUCapsVirtioPCIObjects },
+{ "ats",
+  QEMU_CAPS_VIRTIO_PCI_ATS,
+  virQEMUCapsVirtioPCIObjects },
 };
 
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 275bbcf..e57cae9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -413,6 +413,8 @@ typedef enum {
 
 /* 260 */
 QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB, /* intel-iommu.device-iotlb */
+QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM, /* virtio-*-pci.iommu_platform */
+QEMU_CAPS_VIRTIO_PCI_ATS, /* virtio-*-pci.ats */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1f54ba0..59da13f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -389,6 +389,38 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
 }
 
 static int
+qemuBuildVirtioOptionsStr(virBufferPtr buf,
+  virDomainVirtioOptionsPtr virtio,
+  virQEMUCapsPtr qemuCaps)
+{
+if (!virtio)
+return 0;
+
+if (virtio->iommu_platform != VIR_TRISTATE_SWITCH_ABSENT) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("the iommu_platform setting is not supported "
+ "with this QEMU binary"));
+return -1;
+}
+virBufferAsprintf(buf, ",iommu_platform=%s",
+  
virTristateSwitchTypeToString(virtio->iommu_platform));
+}
+if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_ATS)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("the ats setting is not supported with this "
+ "QEMU binary"));
+return -1;
+}
+virBufferAsprintf(buf, ",ats=%s",
+  virTristateSwitchTypeToString(virtio->ats));
+}
+
+return 0;
+}
+
+static int
 qemuBuildRomStr(virBufferPtr buf,
 virDomainDeviceInfoPtr info)
 {
@@ -2167,6 +2199,10 @@ qemuBuildDriveDevStr(const virDomainDef *def,
   (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN)
   ? "on" : "off");
 }
+
+if (qemuBuildVirtioOptionsStr(, disk->virtio, qemuCaps) < 0)
+goto error;
+
 if (qemuBuildDeviceAddressStr(, def, >info, qemuCaps) < 0)
 goto error;
 break;
@@ -2490,6 +2526,8 @@ qemuBuildFSDevStr(const virDomainDef *def,
   QEMU_FSDEV_HOST_PREFIX, fs->info.alias);
 virBufferAsprintf(, ",mount_tag=%s", fs->dst);
 
+qemuBuildVirtioOptionsStr(, fs->virtio, qemuCaps);
+
 if (qemuBuildDeviceAddressStr(, def, >info, qemuCaps) < 0)
 goto error;
 
@@ -2734,6 +2772,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
   def->iothread);
 }
 }
+if (qemuBuildVirtioOptionsStr(, def->virtio, qemuCaps) < 0)
+goto error;
 break;
 case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
 virBufferAddLit(, "lsi");
@@ -2779,6 +2819,8 @@ qemuBuildControllerDevStr(const 

[libvirt] [PATCH 00/18] Allow virtio devices to use vIOMMU

[Ján Tomko]

[Adding Jason to cc - could you please take a look at the documentation
(patches 6 and 8) and point out any device/option combinations that
do not make sense?]

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

Ján Tomko (18):
  conf: introduce virDomainControllerDriverFormat
  conf: use a separate buffer for the subelements of 
  conf: only format  as a pair tag when needed
  conf: eliminate monster condition in virDomainControllerDefFormat
  virDomainControllerDefFormat: move PCI model and target formatting
  conf: add device_iotlb attribute to iommu
  qemu: format device-iotlb on intel-iommu command line
  qemuxml2xmltest: add virtio-options test
  Add virtio-related options to interfaces
  add virtio-related options to memballoon
  Add virtio-related options to disks
  Add virtio-related options to controllers
  Add virtio-related options to filesystems
  Add virtio-related options to rng devices
  Add virtio-related options to video
  Add virtio-related options to input devices
  qemuxml2argvtest: add virtio-options test case
  qemu: format virtio-related options on the command line

 docs/formatdomain.html.in  |  87 ++
 docs/schemas/domaincommon.rng  |  40 +++
 src/conf/domain_conf.c | 314 -
 src/conf/domain_conf.h |  27 ++
 src/qemu/qemu_capabilities.c   |  15 +-
 src/qemu/qemu_capabilities.h   |   5 +
 src/qemu/qemu_command.c|  69 +
 tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |   3 +
 .../qemuxml2argv-intel-iommu-device-iotlb.args |  19 ++
 .../qemuxml2argv-intel-iommu-device-iotlb.xml  |  31 ++
 .../qemuxml2argv-virtio-options.args   |  57 
 .../qemuxml2argv-virtio-options.xml| 104 +++
 tests/qemuxml2argvtest.c   |  18 ++
 .../qemuxml2xmlout-intel-iommu-device-iotlb.xml|   1 +
 .../qemuxml2xmlout-s390-defaultconsole.xml |   3 +-
 .../qemuxml2xmlout-virtio-options.xml  | 104 +++
 tests/qemuxml2xmltest.c|   2 +
 17 files changed, 825 insertions(+), 74 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
 create mode 12 
tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml

-- 
2.10.2

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

[libvirt] [PATCH 03/18] conf: only format as a pair tag when needed

[Ján Tomko]

Make the decision based on the usage of childBuf buffer.

This fixes the oddity in the test case introduced by commit c1c4d0d
where we would format an empty pair tag.
---
 src/conf/domain_conf.c  | 4 +++-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml | 3 +--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fe0eaf2..57f52be 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21464,7 +21464,6 @@ virDomainControllerDefFormat(virBufferPtr buf,
 def->queues || def->cmd_per_lun || def->max_sectors || def->ioeventfd 
||
 def->iothread ||
 virDomainDeviceInfoNeedsFormat(>info, flags) || pcihole64) {
-virBufferAddLit(buf, ">\n");
 
 if (pciModel) {
 modelName = 
virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName);
@@ -21513,7 +21512,10 @@ virDomainControllerDefFormat(virBufferPtr buf,
 virBufferAsprintf(, "%lu\n", def->opts.pciopts.pcihole64size);
 }
+}
 
+if (virBufferUse()) {
+virBufferAddLit(buf, ">\n");
 virBufferAddBuffer(buf, );
 virBufferAddLit(buf, "\n");
 } else {
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml
index 882d005..7eb1a76 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml
@@ -14,8 +14,7 @@
   destroy
   
 /usr/bin/qemu-system-s390x
-
-
+
 
   
 
-- 
2.10.2

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

[libvirt] [PATCH 15/18] Add virtio-related options to video

[Ján Tomko]

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 docs/formatdomain.html.in  | 12 +++
 docs/schemas/domaincommon.rng  |  5 +
 src/conf/domain_conf.c | 24 --
 src/conf/domain_conf.h |  1 +
 .../qemuxml2argv-virtio-options.xml|  1 +
 .../qemuxml2xmlout-virtio-options.xml  |  1 +
 6 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a546cbb..5c0f3bf 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6136,6 +6136,18 @@ qemu-kvm -net nic,model=? /dev/null
 The optional address sub-element can be used to
 tie the video device to a particular PCI slot.
   
+
+  driver
+  
+The subelement driver can be used to tune the device:
+
+  virtio options
+  
+  Virtio-specific options can also be
+  set. (Since 3.5.0)
+  
+
+  
 
 
 Consoles, serial, parallel  channel 
devices
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3909a56..3df4fa4 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3219,6 +3219,11 @@
   
 
   
+
+  
+
+  
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9604078..8ef50d7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2331,6 +2331,7 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def)
 virDomainDeviceInfoClear(>info);
 
 VIR_FREE(def->accel);
+VIR_FREE(def->virtio);
 VIR_FREE(def);
 }
 
@@ -13471,11 +13472,13 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
 
 static virDomainVideoDefPtr
 virDomainVideoDefParseXML(xmlNodePtr node,
+  xmlXPathContextPtr ctxt,
   const virDomainDef *dom,
   unsigned int flags)
 {
 virDomainVideoDefPtr def;
 xmlNodePtr cur;
+xmlNodePtr saved = ctxt->node;
 char *type = NULL;
 char *heads = NULL;
 char *vram = NULL;
@@ -13484,6 +13487,8 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 char *vgamem = NULL;
 char *primary = NULL;
 
+ctxt->node = node;
+
 if (VIR_ALLOC(def) < 0)
 return NULL;
 
@@ -13585,7 +13590,12 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 if (virDomainDeviceInfoParseXML(node, NULL, >info, flags) < 0)
 goto error;
 
+if (virDomainVirtioOptionsParseXML(ctxt, >virtio) < 0)
+goto error;
+
  cleanup:
+ctxt->node = saved;
+
 VIR_FREE(type);
 VIR_FREE(ram);
 VIR_FREE(vram);
@@ -14384,7 +14394,7 @@ virDomainDeviceDefParse(const char *xmlStr,
 goto error;
 break;
 case VIR_DOMAIN_DEVICE_VIDEO:
-if (!(dev->data.video = virDomainVideoDefParseXML(node, def, flags)))
+if (!(dev->data.video = virDomainVideoDefParseXML(node, ctxt, def, 
flags)))
 goto error;
 break;
 case VIR_DOMAIN_DEVICE_HOSTDEV:
@@ -18327,7 +18337,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 virDomainVideoDefPtr video;
 ssize_t insertAt = -1;
 
-if (!(video = virDomainVideoDefParseXML(nodes[i], def, flags)))
+if (!(video = virDomainVideoDefParseXML(nodes[i], ctxt, def, flags)))
 goto error;
 
 if (video->primary) {
@@ -23278,6 +23288,7 @@ virDomainVideoDefFormat(virBufferPtr buf,
 unsigned int flags)
 {
 const char *model = virDomainVideoTypeToString(def->type);
+virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
 
 if (!model) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -23287,6 +23298,15 @@ virDomainVideoDefFormat(virBufferPtr buf,
 
 virBufferAddLit(buf, "\n");
 virBufferAdjustIndent(buf, 2);
+virDomainVirtioOptionsFormat(, def->virtio);
+if (virBufferCheckError() < 0)
+return -1;
+if (virBufferUse()) {
+virBufferTrim(, " ", -1);
+virBufferAddLit(buf, "\n");
+}
 virBufferAsprintf(buf, "ram)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9f4fe9c..5e38d0f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1383,6 +1383,7 @@ struct _virDomainVideoDef {
 bool primary;
 virDomainVideoAccelDefPtr accel;
 virDomainDeviceInfo info;
+virDomainVirtioOptionsPtr virtio;
 };
 
 /* graphics console modes */
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
index e852894..be702cb 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
@@ -81,6 +81,7 @@
 
 
 
+  
   
 
   
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml 

[libvirt] [PATCH 11/18] Add virtio-related options to disks

[Ján Tomko]

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 docs/formatdomain.html.in  | 5 +
 docs/schemas/domaincommon.rng  | 1 +
 src/conf/domain_conf.c | 9 +
 src/conf/domain_conf.h | 1 +
 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 4 ++--
 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 4 ++--
 6 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c286f50..6852f49 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3011,6 +3011,11 @@
 bus and "pci" or "ccw" address types.
 Since 1.2.8 (QEMU 2.1)
   
+  
+  For virtio disks,
+  Virtio-specific options can also be
+  set. (Since 3.5.0)
+  
 
   
   backenddomain
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5b424c7..a77a04f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1770,6 +1770,7 @@
   
 
   
+  
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ae7cb14..b81743e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1724,6 +1724,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def)
 VIR_FREE(def->product);
 VIR_FREE(def->domain_name);
 VIR_FREE(def->blkdeviotune.group_name);
+VIR_FREE(def->virtio);
 virDomainDeviceInfoClear(>info);
 virObjectUnref(def->privateData);
 
@@ -8397,6 +8398,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 }
 
+if (virDomainVirtioOptionsParseXML(ctxt, >virtio) < 0)
+goto error;
+
 /* Disk volume types will have authentication information handled in
  * virStorageTranslateDiskSourcePool
  */
@@ -21272,6 +21276,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
 virBufferAsprintf(, " iothread='%u'", def->iothread);
 if (def->detect_zeroes)
 virBufferAsprintf(, " detect_zeroes='%s'", detect_zeroes);
+
+virBufferAddLit(, " ");
+virDomainVirtioOptionsFormat(, def->virtio);
+virBufferTrim(, " ", -1);
+
 if (virBufferUse()) {
 virBufferAddLit(buf, " 0 specific thread # */
 int detect_zeroes; /* enum virDomainDiskDetectZeroes */
 char *domain_name; /* backend domain name */
+virDomainVirtioOptionsPtr virtio;
 };
 
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
index 96ec700..25a524a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
@@ -15,13 +15,13 @@
   
 /usr/bin/qemu-system-x86_64
 
-  
+  
   
   
   
 
 
-  
+  
   
   
   
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
index 96ec700..25a524a 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
@@ -15,13 +15,13 @@
   
 /usr/bin/qemu-system-x86_64
 
-  
+  
   
   
   
 
 
-  
+  
   
   
   
-- 
2.10.2

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

[libvirt] [PATCH 10/18] add virtio-related options to memballoon

[Ján Tomko]

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 docs/formatdomain.html.in   |  7 +++
 docs/schemas/domaincommon.rng   |  5 +
 src/conf/domain_conf.c  | 21 +
 src/conf/domain_conf.h  |  1 +
 .../qemuxml2argv-virtio-options.xml |  1 +
 .../qemuxml2xmlout-virtio-options.xml   |  1 +
 6 files changed, 36 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index dcc2e5e..c286f50 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6916,6 +6916,7 @@ qemu-kvm -net nic,model=? /dev/null
 memballoon model='virtio'
   address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
   stats period='10'/
+  driver iommu_platform='on' ats='on'/
 /memballoon
   /devices
 /domain
@@ -6960,6 +6961,12 @@ qemu-kvm -net nic,model=? /dev/null
   Since 1.1.1, requires QEMU 1.5
 
   
+  driver
+  
+For model virtio memballoon,
+Virtio-specific options can also be
+set. (Since 3.5.0)
+  
 
 Random number generator device
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index d7f3b02..5b424c7 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3784,6 +3784,11 @@
 
   
 
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a83f7dc..ae7cb14 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2284,6 +2284,7 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr 
def)
 return;
 
 virDomainDeviceInfoClear(>info);
+VIR_FREE(def->virtio);
 
 VIR_FREE(def);
 }
@@ -12947,6 +12948,9 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
 else if (virDomainDeviceInfoParseXML(node, NULL, >info, flags) < 0)
 goto error;
 
+if (virDomainVirtioOptionsParseXML(ctxt, >virtio) < 0)
+goto error;
+
  cleanup:
 VIR_FREE(model);
 VIR_FREE(deflate);
@@ -22896,6 +22900,23 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
 return -1;
 }
 
+if (def->virtio) {
+virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
+
+virDomainVirtioOptionsFormat(, def->virtio);
+virBufferTrim(, " ", -1);
+
+if (virBufferCheckError() < 0) {
+virBufferFreeAndReset();
+return -1;
+}
+if (virBufferUse()) {
+virBufferAddLit(, "\n");
+}
+}
+
 if (!virBufferUse()) {
 virBufferAddLit(buf, "/>\n");
 } else {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 68f8d0c..3a95c64 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1616,6 +1616,7 @@ struct _virDomainMemballoonDef {
 virDomainDeviceInfo info;
 int period; /* seconds between collections */
 int autodeflate; /* enum virTristateSwitch */
+virDomainVirtioOptionsPtr virtio;
 };
 
 struct _virDomainNVRAMDef {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
index a2bbbee..96ec700 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
@@ -85,6 +85,7 @@
 
 
   
+  
 
 
   /dev/random
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
index a2bbbee..96ec700 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
@@ -85,6 +85,7 @@
 
 
   
+  
 
 
   /dev/random
-- 
2.10.2

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

[libvirt] [PATCH 12/18] Add virtio-related options to controllers

[Ján Tomko]

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 docs/formatdomain.html.in  | 6 ++
 docs/schemas/domaincommon.rng  | 1 +
 src/conf/domain_conf.c | 8 
 src/conf/domain_conf.h | 1 +
 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 2 ++
 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 2 ++
 6 files changed, 20 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6852f49..86e0bf8 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3600,6 +3600,12 @@
 iothread value. The iothread value
 must be within the range 1 to the domain iothreads value.
   
+  virtio options
+  
+For virtio controllers,
+Virtio-specific options can also be
+set. (Since 3.5.0)
+  
 
 
   USB companion controllers have an optional
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index a77a04f..01c35ae 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2073,6 +2073,7 @@
 
   
 
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b81743e..e4b5ca1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1865,6 +1865,7 @@ void virDomainControllerDefFree(virDomainControllerDefPtr 
def)
 return;
 
 virDomainDeviceInfoClear(>info);
+VIR_FREE(def->virtio);
 
 VIR_FREE(def);
 }
@@ -9010,6 +9011,9 @@ virDomainControllerDefParseXML(xmlNodePtr node,
 cur = cur->next;
 }
 
+if (virDomainVirtioOptionsParseXML(ctxt, >virtio) < 0)
+goto error;
+
 /* node is parsed differently from target attributes because
  * someone thought it should be a subelement instead...
  */
@@ -21463,6 +21467,10 @@ virDomainControllerDriverFormat(virBufferPtr buf,
 if (def->iothread)
 virBufferAsprintf(, " iothread='%u'", def->iothread);
 
+virBufferAddLit(, " ");
+virDomainVirtioOptionsFormat(, def->virtio);
+virBufferTrim(, " ", -1);
+
 if (virBufferUse()) {
 virBufferAddLit(buf, "

[libvirt] [PATCH 09/18] Add virtio-related options to interfaces

[Ján Tomko]

  
  
  


https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 docs/formatdomain.html.in  | 19 +++
 docs/schemas/domaincommon.rng  | 12 +
 src/conf/domain_conf.c | 63 ++
 src/conf/domain_conf.h | 19 +++
 .../qemuxml2argv-virtio-options.xml|  2 +
 .../qemuxml2xmlout-virtio-options.xml  |  2 +
 6 files changed, 117 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 2f1e030..dcc2e5e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3451,6 +3451,19 @@
   
 
 
+Virtio-related options
+
+
+  QEMU's virtio devices have some attributes related to the virtio 
transport under
+  the driver element:
+  The iommu_platform attribute enables the use of emulated 
IOMMU
+  by the device. The attribute ats controls the Address
+  Translation Service support for PCIe devices. This is needed to make use
+  of IOTLB support (see IOMMU device).
+  Possible values are on or off.
+  Since 3.5.0
+
+
 Controllers
 
 
@@ -5142,6 +5155,12 @@ qemu-kvm -net nic,model=? /dev/null
 In general you should leave this option alone, unless you
 are very certain you know what you are doing.
   
+  virtio options
+  
+For virtio interfaces,
+Virtio-specific options can also be
+set. (Since 3.5.0)
+  
 
 
   Offloading options for the host and guest can be configured using
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6c3e885..d7f3b02 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2686,6 +2686,7 @@
   
 
   
+  
   
 
   
@@ -5006,6 +5007,17 @@
 
   
 
+  
+
+  
+
+  
+  
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c83af5d..a83f7dc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1098,6 +1098,46 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr 
xmlopt)
 return >ns;
 }
 
+static int
+virDomainVirtioOptionsParseXML(xmlXPathContextPtr ctxt,
+   virDomainVirtioOptionsPtr *virtio)
+{
+char *str = NULL;
+int ret = -1;
+int val;
+virDomainVirtioOptionsPtr res;
+
+if (VIR_ALLOC(*virtio) < 0)
+return -1;
+
+res = *virtio;
+
+if ((str = virXPathString("string(./driver/@iommu_platform)", ctxt))) {
+if ((val = virTristateSwitchTypeFromString(str)) <= 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("invalid iommu_platform value"));
+goto cleanup;
+}
+res->iommu_platform = val;
+}
+VIR_FREE(str);
+
+if ((str = virXPathString("string(./driver/@ats)", ctxt))) {
+if ((val = virTristateSwitchTypeFromString(str)) <= 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("invalid ats value"));
+goto cleanup;
+}
+res->ats = val;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(str);
+return ret;
+}
+
 
 void
 virBlkioDeviceArrayClear(virBlkioDevicePtr devices,
@@ -1945,6 +1985,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
 VIR_FREE(def->ifname);
 VIR_FREE(def->ifname_guest);
 VIR_FREE(def->ifname_guest_actual);
+VIR_FREE(def->virtio);
 
 virNetDevIPInfoClear(>guestIP);
 virNetDevIPInfoClear(>hostIP);
@@ -5214,6 +5255,24 @@ virDomainDefValidate(virDomainDefPtr def,
 }
 
 
+static void
+virDomainVirtioOptionsFormat(virBufferPtr buf,
+ virDomainVirtioOptionsPtr virtio)
+{
+if (!virtio)
+return;
+
+if (virtio->iommu_platform != VIR_TRISTATE_SWITCH_ABSENT) {
+virBufferAsprintf(buf, "iommu_platform='%s' ",
+  
virTristateSwitchTypeToString(virtio->iommu_platform));
+}
+if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
+virBufferAsprintf(buf, "ats='%s' ",
+  virTristateSwitchTypeToString(virtio->ats));
+}
+}
+
+
 /* Generate a string representation of a device address
  * @info address Device address to stringify
  */
@@ -10360,6 +10419,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 goto error;
 }
 
+if (virDomainVirtioOptionsParseXML(ctxt, >virtio) < 0)
+goto error;
+
  cleanup:
 ctxt->node = oldnode;
 VIR_FREE(macaddr);
@@ -22092,6 +22154,7 @@ virDomainVirtioNetDriverFormat(char **outstr,
 virBufferAsprintf(, "rx_queue_size='%u' ",
   def->driver.virtio.rx_queue_size);
 
+virDomainVirtioOptionsFormat(, def->virtio);
 virBufferTrim(, " ", -1);
 
 if (virBufferCheckError() < 0)
diff --git 

[libvirt] [PATCH 17/18] qemuxml2argvtest: add virtio-options test case

[Ján Tomko]

Add a test case to demonstrate the addition of new command line options

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 .../qemuxml2argv-virtio-options.args   | 47 ++
 tests/qemuxml2argvtest.c   |  9 +
 2 files changed, 56 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args 
b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args
new file mode 100644
index 000..b8b5910
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args
@@ -0,0 +1,47 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x8 \
+-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x9 \
+-usb \
+-drive file=/var/lib/libvirt/images/img1,format=raw,if=none,\
+id=drive-virtio-disk0 \
+-device virtio-blk-pci,bus=pci.0,addr=0xa,drive=drive-virtio-disk0,\
+id=virtio-disk0 \
+-drive file=/var/lib/libvirt/images/img2,format=raw,if=none,\
+id=drive-virtio-disk1 \
+-device virtio-blk-pci,bus=pci.0,addr=0xb,drive=drive-virtio-disk1,\
+id=virtio-disk1 \
+-fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/fs1 \
+-device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=fs1,bus=pci.0,addr=0x3 \
+-fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\
+path=/export/fs2 \
+-device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,mount_tag=fs2,bus=pci.0,addr=0x4 \
+-device virtio-net-pci,vlan=0,id=net0,mac=52:54:56:58:5a:5c,bus=pci.0,addr=0x6 
\
+-net user,vlan=0,name=hostnet0 \
+-device virtio-net-pci,vlan=1,id=net1,mac=52:54:56:5a:5c:5e,bus=pci.0,addr=0x7 
\
+-net user,vlan=1,name=hostnet1 \
+-device virtio-mouse-pci,id=input0,bus=pci.0,addr=0xe \
+-device virtio-keyboard-pci,id=input1,bus=pci.0,addr=0x10 \
+-device virtio-tablet-pci,id=input2,bus=pci.0,addr=0x11 \
+-device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\
+addr=0x12 \
+-device virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc \
+-object rng-random,id=objrng0,filename=/dev/random \
+-device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.0,addr=0xd
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 508d7d9..650e949 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2545,6 +2545,15 @@ mymain(void)
 QEMU_CAPS_DEVICE_INTEL_IOMMU);
 
 DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS);
+DO_TEST("virtio-options", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_KEYBOARD,
+QEMU_CAPS_VIRTIO_MOUSE, QEMU_CAPS_VIRTIO_TABLET,
+QEMU_CAPS_VIRTIO_INPUT_HOST,
+QEMU_CAPS_FSDEV, QEMU_CAPS_FSDEV_WRITEOUT,
+QEMU_CAPS_DEVICE_VIRTIO_GPU,
+QEMU_CAPS_VIRTIO_GPU_VIRGL,
+QEMU_CAPS_DEVICE_VIRTIO_RNG,
+QEMU_CAPS_OBJECT_RNG_RANDOM,
+QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
 
 DO_TEST("fd-memory-numa-topology", QEMU_CAPS_MEM_PATH, 
QEMU_CAPS_OBJECT_MEMORY_FILE,
 QEMU_CAPS_KVM);
-- 
2.10.2

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

[libvirt] [PATCH 14/18] Add virtio-related options to rng devices

[Ján Tomko]

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 docs/formatdomain.html.in| 11 +++
 docs/schemas/domaincommon.rng|  5 +
 src/conf/domain_conf.c   | 16 
 src/conf/domain_conf.h   |  1 +
 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml   |  1 +
 .../qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml |  1 +
 6 files changed, 35 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ef30079..a546cbb 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7064,6 +7064,17 @@ qemu-kvm -net nic,model=? /dev/null
   
 
   
+  driver
+  
+The subelement driver can be used to tune the device:
+
+  virtio options
+  
+  Virtio-specific options can also be
+  set. (Since 3.5.0)
+  
+
+  
 
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3608f84..3909a56 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4966,6 +4966,11 @@
   
 
 
+  
+
+  
+
+
   
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2bbecea..9604078 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12895,6 +12895,9 @@ virDomainRNGDefParseXML(virDomainXMLOptionPtr xmlopt,
 if (virDomainDeviceInfoParseXML(node, NULL, >info, flags) < 0)
 goto error;
 
+if (virDomainVirtioOptionsParseXML(ctxt, >virtio) < 0)
+goto error;
+
  cleanup:
 VIR_FREE(model);
 VIR_FREE(backend);
@@ -23081,6 +23084,7 @@ virDomainRNGDefFormat(virBufferPtr buf,
 {
 const char *model = virDomainRNGModelTypeToString(def->model);
 const char *backend = virDomainRNGBackendTypeToString(def->backend);
+virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
 
 virBufferAsprintf(buf, "\n", model);
 virBufferAdjustIndent(buf, 2);
@@ -23109,6 +23113,17 @@ virDomainRNGDefFormat(virBufferPtr buf,
 break;
 }
 
+virDomainVirtioOptionsFormat(, def->virtio);
+if (virBufferCheckError() < 0)
+return -1;
+
+if (virBufferUse()) {
+virBufferTrim(, " ", -1);
+virBufferAddLit(buf, "\n");
+}
+
 if (virDomainDeviceInfoNeedsFormat(>info, flags)) {
 if (virDomainDeviceInfoFormat(buf, >info, flags) < 0)
 return -1;
@@ -23137,6 +23152,7 @@ virDomainRNGDefFree(virDomainRNGDefPtr def)
 }
 
 virDomainDeviceInfoClear(>info);
+VIR_FREE(def->virtio);
 VIR_FREE(def);
 }
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cb27538..9f4fe9c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2027,6 +2027,7 @@ struct _virDomainRNGDef {
 } source;
 
 virDomainDeviceInfo info;
+virDomainVirtioOptionsPtr virtio;
 };
 
 typedef enum {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
index 4fc17d2..e852894 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
@@ -92,6 +92,7 @@
 
 
   /dev/random
+  
   
 
   
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
index 4fc17d2..e852894 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
@@ -92,6 +92,7 @@
 
 
   /dev/random
+  
   
 
   
-- 
2.10.2

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

[libvirt] [PATCH 16/18] Add virtio-related options to input devices

[Ján Tomko]

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 docs/formatdomain.html.in | 13 +
 docs/schemas/domaincommon.rng |  5 +
 src/conf/domain_conf.c| 15 +++
 src/conf/domain_conf.h|  1 +
 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml|  4 
 .../qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml  |  4 
 6 files changed, 42 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 5c0f3bf..c9c3592 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5715,6 +5715,19 @@ qemu-kvm -net nic,model=? /dev/null
   event device passed through to guests. (KVM only)
 
 
+
+The subelement driver can be used to tune the virtio
+options of the device:
+Virtio-specific options can also be
+set. (Since 3.5.0)
+
+
+  The compatibility attribute of the driver 
element
+  can be used to specify the compatibility of virtio devices. Allowed 
values
+  are legacy, transitional and 
modern.
+  Since 2.2.0.
+
+
 Hub devices
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3df4fa4..78ff0a2 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3990,6 +3990,11 @@
 
   
 
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8ef50d7..5dc3e5c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1379,6 +1379,7 @@ void virDomainInputDefFree(virDomainInputDefPtr def)
 
 virDomainDeviceInfoClear(>info);
 VIR_FREE(def->source.evdev);
+VIR_FREE(def->virtio);
 VIR_FREE(def);
 }
 
@@ -11560,6 +11561,9 @@ virDomainInputDefParseXML(const virDomainDef *dom,
 goto error;
 }
 
+if (virDomainVirtioOptionsParseXML(ctxt, >virtio) < 0)
+goto error;
+
  cleanup:
 VIR_FREE(evdev);
 VIR_FREE(type);
@@ -23347,6 +23351,7 @@ virDomainInputDefFormat(virBufferPtr buf,
 const char *type = virDomainInputTypeToString(def->type);
 const char *bus = virDomainInputBusTypeToString(def->bus);
 virBuffer childbuf = VIR_BUFFER_INITIALIZER;
+virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
 
 /* don't format keyboard into migratable XML for backward compatibility */
 if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE &&
@@ -23370,6 +23375,16 @@ virDomainInputDefFormat(virBufferPtr buf,
   type, bus);
 
 virBufferAdjustIndent(, virBufferGetIndent(buf, false) + 2);
+virDomainVirtioOptionsFormat(, def->virtio);
+if (virBufferCheckError() < 0)
+return -1;
+
+if (virBufferUse()) {
+virBufferTrim(, " ", -1);
+virBufferAddLit(, "\n");
+}
 virBufferEscapeString(, "\n", 
def->source.evdev);
 if (virDomainDeviceInfoFormat(, >info, flags) < 0)
 return -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5e38d0f..0038a11 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1289,6 +1289,7 @@ struct _virDomainInputDef {
 char *evdev;
 } source;
 virDomainDeviceInfo info;
+virDomainVirtioOptionsPtr virtio;
 };
 
 typedef enum {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
index be702cb..f614e68 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
@@ -66,15 +66,19 @@
   
 
 
+  
   
 
 
+  
   
 
 
+  
   
 
 
+  
   
   
 
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
index be702cb..f614e68 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
@@ -66,15 +66,19 @@
   
 
 
+  
   
 
 
+  
   
 
 
+  
   
 
 
+  
   
   
 
-- 
2.10.2

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

[libvirt] [PATCH 01/18] conf: introduce virDomainControllerDriverFormat

[Ján Tomko]

Split out formatting the  subelement of 
to make adding new options easier.
---
 src/conf/domain_conf.c | 55 +-
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c7e20b8..318e254 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21357,6 +21357,37 @@ virDomainDiskDefFormat(virBufferPtr buf,
 #undef FORMAT_IOTUNE
 
 
+static void
+virDomainControllerDriverFormat(virBufferPtr buf,
+virDomainControllerDefPtr def)
+{
+virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
+
+if (def->queues)
+virBufferAsprintf(, " queues='%u'", def->queues);
+
+if (def->cmd_per_lun)
+virBufferAsprintf(, " cmd_per_lun='%u'", def->cmd_per_lun);
+
+if (def->max_sectors)
+virBufferAsprintf(, " max_sectors='%u'", def->max_sectors);
+
+if (def->ioeventfd) {
+virBufferAsprintf(, " ioeventfd='%s'",
+  virTristateSwitchTypeToString(def->ioeventfd));
+}
+
+if (def->iothread)
+virBufferAsprintf(, " iothread='%u'", def->iothread);
+
+if (virBufferUse()) {
+virBufferAddLit(buf, "\n");
+}
+}
+
+
 static int
 virDomainControllerDefFormat(virBufferPtr buf,
  virDomainControllerDefPtr def,
@@ -21366,7 +21397,6 @@ virDomainControllerDefFormat(virBufferPtr buf,
 const char *model = NULL;
 const char *modelName = NULL;
 bool pcihole64 = false, pciModel = false, pciTarget = false;
-virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
 
 if (!type) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -21471,28 +21501,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
 }
 }
 
-if (def->queues)
-virBufferAsprintf(, " queues='%u'", def->queues);
-
-if (def->cmd_per_lun)
-virBufferAsprintf(, " cmd_per_lun='%u'", 
def->cmd_per_lun);
-
-if (def->max_sectors)
-virBufferAsprintf(, " max_sectors='%u'", 
def->max_sectors);
-
-if (def->ioeventfd) {
-virBufferAsprintf(, " ioeventfd='%s'",
-  virTristateSwitchTypeToString(def->ioeventfd));
-}
-
-if (def->iothread)
-virBufferAsprintf(, " iothread='%u'", def->iothread);
-
-if (virBufferUse()) {
-virBufferAddLit(buf, "\n");
-}
+virDomainControllerDriverFormat(buf, def);
 
 if (virDomainDeviceInfoNeedsFormat(>info, flags) &&
 virDomainDeviceInfoFormat(buf, >info, flags) < 0)
-- 
2.10.2

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

[libvirt] [PATCH 13/18] Add virtio-related options to filesystems

[Ján Tomko]

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 docs/formatdomain.html.in  | 5 +
 docs/schemas/domaincommon.rng  | 1 +
 src/conf/domain_conf.c | 8 
 src/conf/domain_conf.h | 1 +
 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 3 ++-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 3 ++-
 6 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 86e0bf8..ef30079 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3296,6 +3296,11 @@
 or "handle", but no formats. Virtuozzo driver supports
 a type of "ploop" with a format of "ploop".
   
+  
+  For virtio-backed devices,
+  Virtio-specific options can also be
+  set. (Since 3.5.0)
+  
 
   
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 01c35ae..3608f84 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2256,6 +2256,7 @@
   immediate
 
   
+  
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e4b5ca1..2bbecea 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1897,6 +1897,7 @@ void virDomainFSDefFree(virDomainFSDefPtr def)
 virStorageSourceFree(def->src);
 VIR_FREE(def->dst);
 virDomainDeviceInfoClear(>info);
+VIR_FREE(def->virtio);
 
 VIR_FREE(def);
 }
@@ -9424,6 +9425,9 @@ virDomainFSDefParseXML(xmlNodePtr node,
 goto error;
 }
 
+if (virDomainVirtioOptionsParseXML(ctxt, >virtio) < 0)
+goto error;
+
 def->src->path = source;
 source = NULL;
 def->dst = target;
@@ -21662,6 +21666,10 @@ virDomainFSDefFormat(virBufferPtr buf,
 
 }
 
+virBufferAddLit(, " ");
+virDomainVirtioOptionsFormat(, def->virtio);
+virBufferTrim(, " ", -1);
+
 if (virBufferUse()) {
 virBufferAddLit(buf, "

[libvirt] [PATCH 02/18] conf: use a separate buffer for the subelements of

[Ján Tomko]

We need to decide whether to format  as a single tag
or if it has any subelements.

Rewrite the function to use a separate buffer for subelements,
to make adding new options easier.
---
 src/conf/domain_conf.c | 36 +++-
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 318e254..fe0eaf2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21397,6 +21397,9 @@ virDomainControllerDefFormat(virBufferPtr buf,
 const char *model = NULL;
 const char *modelName = NULL;
 bool pcihole64 = false, pciModel = false, pciTarget = false;
+virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+
+virBufferAdjustIndent(, virBufferGetIndent(buf, false) + 2);
 
 if (!type) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -21462,7 +21465,6 @@ virDomainControllerDefFormat(virBufferPtr buf,
 def->iothread ||
 virDomainDeviceInfoNeedsFormat(>info, flags) || pcihole64) {
 virBufferAddLit(buf, ">\n");
-virBufferAdjustIndent(buf, 2);
 
 if (pciModel) {
 modelName = 
virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName);
@@ -21472,47 +21474,47 @@ virDomainControllerDefFormat(virBufferPtr buf,
def->opts.pciopts.modelName);
 return -1;
 }
-virBufferAsprintf(buf, "\n", modelName);
+virBufferAsprintf(, "\n", modelName);
 }
 
 if (pciTarget) {
-virBufferAddLit(buf, "opts.pciopts.chassisNr != -1)
-virBufferAsprintf(buf, " chassisNr='%d'",
+virBufferAsprintf(, " chassisNr='%d'",
   def->opts.pciopts.chassisNr);
 if (def->opts.pciopts.chassis != -1)
-virBufferAsprintf(buf, " chassis='%d'",
+virBufferAsprintf(, " chassis='%d'",
   def->opts.pciopts.chassis);
 if (def->opts.pciopts.port != -1)
-virBufferAsprintf(buf, " port='0x%x'",
+virBufferAsprintf(, " port='0x%x'",
   def->opts.pciopts.port);
 if (def->opts.pciopts.busNr != -1)
-virBufferAsprintf(buf, " busNr='%d'",
+virBufferAsprintf(, " busNr='%d'",
   def->opts.pciopts.busNr);
 if (def->opts.pciopts.numaNode == -1) {
-virBufferAddLit(buf, "/>\n");
+virBufferAddLit(, "/>\n");
 } else {
-virBufferAddLit(buf, ">\n");
-virBufferAdjustIndent(buf, 2);
-virBufferAsprintf(buf, "%d\n",
+virBufferAddLit(, ">\n");
+virBufferAdjustIndent(, 2);
+virBufferAsprintf(, "%d\n",
   def->opts.pciopts.numaNode);
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(, -2);
+virBufferAddLit(, "\n");
 }
 }
 
-virDomainControllerDriverFormat(buf, def);
+virDomainControllerDriverFormat(, def);
 
 if (virDomainDeviceInfoNeedsFormat(>info, flags) &&
-virDomainDeviceInfoFormat(buf, >info, flags) < 0)
+virDomainDeviceInfoFormat(, >info, flags) < 0)
 return -1;
 
 if (pcihole64) {
-virBufferAsprintf(buf, "%lu%lu\n", def->opts.pciopts.pcihole64size);
 }
 
-virBufferAdjustIndent(buf, -2);
+virBufferAddBuffer(buf, );
 virBufferAddLit(buf, "\n");
 } else {
 virBufferAddLit(buf, "/>\n");
-- 
2.10.2

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

[libvirt] [PATCH 08/18] qemuxml2xmltest: add virtio-options test

[Ján Tomko]

Add a test case with all the virtio devices we know to demonstrate
the addition of new options.

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 .../qemuxml2argv-virtio-options.xml| 92 ++
 .../qemuxml2xmlout-virtio-options.xml  | 92 ++
 tests/qemuxml2xmltest.c|  1 +
 3 files changed, 185 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
new file mode 100644
index 000..9ead938
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml
@@ -0,0 +1,92 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+  
+  
+  
+  
+
+
+  
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+
+
+
+  
+
+
+  
+  
+  
+
+
+  
+  
+  
+  
+
+
+  
+  
+  
+
+
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+
+
+  
+  
+
+
+
+
+  
+
+  
+  
+
+
+  
+
+
+  /dev/random
+  
+
+  
+
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
new file mode 100644
index 000..9ead938
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
@@ -0,0 +1,92 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+  
+  
+  
+  
+
+
+  
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+
+
+
+  
+
+
+  
+  
+  
+
+
+  
+  
+  
+  
+
+
+  
+  
+  
+
+
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+
+
+  
+  
+
+
+
+
+  
+
+  
+  
+
+
+  
+
+
+  /dev/random
+  
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 5e8cead..08698b1 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1106,6 +1106,7 @@ mymain(void)
 
 DO_TEST("memorybacking-set", NONE);
 DO_TEST("memorybacking-unset", NONE);
+DO_TEST("virtio-options", QEMU_CAPS_VIRTIO_SCSI);
 
 virObjectUnref(cfg);
 
-- 
2.10.2

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

[libvirt] [PATCH 07/18] qemu: format device-iotlb on intel-iommu command line

[Ján Tomko]

Format the device-iotlb attribute.

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 src/qemu/qemu_capabilities.c  |  3 +++
 src/qemu/qemu_capabilities.h  |  3 +++
 src/qemu/qemu_command.c   | 11 +++
 tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml  |  1 +
 .../qemuxml2argv-intel-iommu-device-iotlb.args| 19 +++
 tests/qemuxml2argvtest.c  |  7 +++
 6 files changed, 44 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7ea8505..ae466d7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -372,6 +372,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "intel-iommu.intremap",
   "intel-iommu.caching-mode",
   "intel-iommu.eim",
+
+  "intel-iommu.device-iotlb", /* 260 */
 );
 
 
@@ -1730,6 +1732,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsIntelIOMMU[] = {
 { "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP },
 { "caching-mode", QEMU_CAPS_INTEL_IOMMU_CACHING_MODE },
 { "eim", QEMU_CAPS_INTEL_IOMMU_EIM },
+{ "device-iotlb", QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB },
 };
 
 /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format */
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index eba9814..275bbcf 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -411,6 +411,9 @@ typedef enum {
 QEMU_CAPS_INTEL_IOMMU_CACHING_MODE, /* intel-iommu.caching-mode */
 QEMU_CAPS_INTEL_IOMMU_EIM, /* intel-iommu.eim */
 
+/* 260 */
+QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB, /* intel-iommu.device-iotlb */
+
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 015af10..1f54ba0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6697,6 +6697,13 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
  "with this QEMU binary"));
 return -1;
 }
+if (iommu->device_iotlb != VIR_TRISTATE_SWITCH_ABSENT &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("iommu: device IOTLB is not supported "
+ "with this QEMU binary"));
+return -1;
+}
 break;
 case VIR_DOMAIN_IOMMU_MODEL_LAST:
 break;
@@ -6734,6 +6741,10 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
 virBufferAsprintf(, ",eim=%s",
   virTristateSwitchTypeToString(iommu->eim));
 }
+if (iommu->device_iotlb != VIR_TRISTATE_SWITCH_ABSENT) {
+virBufferAsprintf(, ",device-iotlb=%s",
+  
virTristateSwitchTypeToString(iommu->device_iotlb));
+}
 case VIR_DOMAIN_IOMMU_MODEL_LAST:
 break;
 }
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
index 95b04dd..d51ee46 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
@@ -215,6 +215,7 @@
   
   
   
+  
   2009000
   0
(v2.9.0)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args 
b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args
new file mode 100644
index 000..6d8f8e2
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args
@@ -0,0 +1,19 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name QEMUGuest1 \
+-S \
+-machine q35,accel=kvm,kernel_irqchip=split \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device intel-iommu,intremap=on,device-iotlb=on
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b360185..508d7d9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2536,6 +2536,13 @@ mymain(void)
 QEMU_CAPS_INTEL_IOMMU_INTREMAP,
 QEMU_CAPS_INTEL_IOMMU_EIM,
 QEMU_CAPS_DEVICE_INTEL_IOMMU);
+DO_TEST("intel-iommu-device-iotlb",
+QEMU_CAPS_MACHINE_OPT,
+QEMU_CAPS_MACHINE_KERNEL_IRQCHIP,
+QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT,
+QEMU_CAPS_INTEL_IOMMU_INTREMAP,
+QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB,
+QEMU_CAPS_DEVICE_INTEL_IOMMU);
 
 DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS);
 
-- 
2.10.2

--

[libvirt] [PATCH 06/18] conf: add device_iotlb attribute to iommu

[Ján Tomko]

Add a new device_iotlb attribute to the iommu device
to control the device IOTLB support for intel-iommu.

https://bugzilla.redhat.com/show_bug.cgi?id=1283251
---
 docs/formatdomain.html.in  |  9 +++
 docs/schemas/domaincommon.rng  |  5 
 src/conf/domain_conf.c | 15 ++-
 src/conf/domain_conf.h |  1 +
 .../qemuxml2argv-intel-iommu-device-iotlb.xml  | 31 ++
 .../qemuxml2xmlout-intel-iommu-device-iotlb.xml|  1 +
 tests/qemuxml2xmltest.c|  1 +
 7 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml
 create mode 12 
tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 07208ee..2f1e030 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7449,6 +7449,15 @@ qemu-kvm -net nic,model=? /dev/null
   Since 3.4.0 (QEMU/KVM only)
 
   
+  device_iotlb
+  
+
+  The device_iotlb attribute with possible values
+  on and off can be used to
+  turn on the device IOTLB descriptor.
+  Since 3.5.0 (QEMU/KVM only)
+
+  
 
   
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4d9f8d1..6c3e885 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3964,6 +3964,11 @@
   
 
   
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b47a376..c83af5d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14201,6 +14201,14 @@ virDomainIOMMUDefParseXML(xmlNodePtr node,
 }
 iommu->caching_mode = val;
 }
+VIR_FREE(tmp);
+if ((tmp = virXPathString("string(./driver/@device_iotlb)", ctxt))) {
+if ((val = virTristateSwitchTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_XML_ERROR, _("unknown device_iotlb value: 
%s"), tmp);
+goto cleanup;
+}
+iommu->device_iotlb = val;
+}
 
 VIR_FREE(tmp);
 if ((tmp = virXPathString("string(./driver/@eim)", ctxt))) {
@@ -24246,7 +24254,8 @@ virDomainIOMMUDefFormat(virBufferPtr buf,
 virBufferAdjustIndent(, virBufferGetIndent(buf, false) + 2);
 
 if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT ||
-iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT) {
+iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT ||
+iommu->device_iotlb != VIR_TRISTATE_SWITCH_ABSENT) {
 virBufferAddLit(, "intremap != VIR_TRISTATE_SWITCH_ABSENT) {
 virBufferAsprintf(, " intremap='%s'",
@@ -24260,6 +24269,10 @@ virDomainIOMMUDefFormat(virBufferPtr buf,
 virBufferAsprintf(, " eim='%s'",
   virTristateSwitchTypeToString(iommu->eim));
 }
+if (iommu->device_iotlb != VIR_TRISTATE_SWITCH_ABSENT) {
+virBufferAsprintf(, " device_iotlb='%s'",
+  
virTristateSwitchTypeToString(iommu->device_iotlb));
+}
 virBufferAddLit(, "/>\n");
 }
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 83e0672..cb570ef 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2212,6 +2212,7 @@ struct _virDomainIOMMUDef {
 virTristateSwitch intremap;
 virTristateSwitch caching_mode;
 virTristateSwitch eim;
+virTristateSwitch device_iotlb;
 };
 /*
  * Guest VM main configuration
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml
new file mode 100644
index 000..0cdf2aa
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml
@@ -0,0 +1,31 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219100
+  219100
+  1
+  
+hvm
+
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+
+  
+
+
+
+
+
+  
+
+  
+
diff --git 
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml
new file mode 12
index 000..3120d9f
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index fff13e2..5e8cead 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1128,6 +1128,7 @@ mymain(void)
 DO_TEST("intel-iommu-ioapic", NONE);
 

[libvirt] [PATCH 04/18] conf: eliminate monster condition in virDomainControllerDefFormat

[Ján Tomko]

Move most of the subelement formatting out of the giant if.
---
 src/conf/domain_conf.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 57f52be..81385d6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21460,11 +21460,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
 break;
 }
 
-if (pciModel || pciTarget ||
-def->queues || def->cmd_per_lun || def->max_sectors || def->ioeventfd 
||
-def->iothread ||
-virDomainDeviceInfoNeedsFormat(>info, flags) || pcihole64) {
-
+if (pciModel || pciTarget) {
 if (pciModel) {
 modelName = 
virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName);
 if (!modelName) {
@@ -21501,17 +21497,17 @@ virDomainControllerDefFormat(virBufferPtr buf,
 virBufferAddLit(, "\n");
 }
 }
+}
 
-virDomainControllerDriverFormat(, def);
+virDomainControllerDriverFormat(, def);
 
-if (virDomainDeviceInfoNeedsFormat(>info, flags) &&
-virDomainDeviceInfoFormat(, >info, flags) < 0)
-return -1;
+if (virDomainDeviceInfoNeedsFormat(>info, flags) &&
+virDomainDeviceInfoFormat(, >info, flags) < 0)
+return -1;
 
-if (pcihole64) {
-virBufferAsprintf(, "%lu\n", def->opts.pciopts.pcihole64size);
-}
+if (pcihole64) {
+virBufferAsprintf(, "%lu\n", def->opts.pciopts.pcihole64size);
 }
 
 if (virBufferUse()) {
-- 
2.10.2

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

[libvirt] [PATCH 05/18] virDomainControllerDefFormat: move PCI model and target formatting

[Ján Tomko]

These can be formatted right when we know we will need them.
No need for separate bool variables.
---
 src/conf/domain_conf.c | 30 +++---
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 81385d6..b47a376 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21396,7 +21396,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
 const char *type = virDomainControllerTypeToString(def->type);
 const char *model = NULL;
 const char *modelName = NULL;
-bool pcihole64 = false, pciModel = false, pciTarget = false;
+bool pcihole64 = false;
 virBuffer childBuf = VIR_BUFFER_INITIALIZER;
 
 virBufferAdjustIndent(, virBufferGetIndent(buf, false) + 2);
@@ -21446,22 +21446,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
 case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
 if (def->opts.pciopts.pcihole64)
 pcihole64 = true;
-if (def->opts.pciopts.modelName != 
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE)
-pciModel = true;
-if (def->opts.pciopts.chassisNr != -1 ||
-def->opts.pciopts.chassis != -1 ||
-def->opts.pciopts.port != -1 ||
-def->opts.pciopts.busNr != -1 ||
-def->opts.pciopts.numaNode != -1)
-pciTarget = true;
-break;
-
-default:
-break;
-}
-
-if (pciModel || pciTarget) {
-if (pciModel) {
+if (def->opts.pciopts.modelName != 
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
 modelName = 
virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName);
 if (!modelName) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -21471,8 +21456,11 @@ virDomainControllerDefFormat(virBufferPtr buf,
 }
 virBufferAsprintf(, "\n", modelName);
 }
-
-if (pciTarget) {
+if (def->opts.pciopts.chassisNr != -1 ||
+def->opts.pciopts.chassis != -1 ||
+def->opts.pciopts.port != -1 ||
+def->opts.pciopts.busNr != -1 ||
+def->opts.pciopts.numaNode != -1) {
 virBufferAddLit(, "opts.pciopts.chassisNr != -1)
 virBufferAsprintf(, " chassisNr='%d'",
@@ -21497,6 +21485,10 @@ virDomainControllerDefFormat(virBufferPtr buf,
 virBufferAddLit(, "\n");
 }
 }
+break;
+
+default:
+break;
 }
 
 virDomainControllerDriverFormat(, def);
-- 
2.10.2

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

Re: [libvirt] [PATCH 4/9] util: Add magic_marker for all virObjects

[Pavel Hrdina]

On Tue, May 30, 2017 at 07:38:16AM -0400, John Ferlan wrote:
> The virObject logic "assumes" that whatever is passed to its API's
> would be some sort of virObjectPtr; however, if it is not then some
> really bad things can happen.
> 
> So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock},
> and virObjectIsClass and the virObject and virObjectLockable class
> consumers have been well behaved and code well tested. Soon there will
> be more consumers and one such consumer tripped over this during testing
> by passing a virHashTablePtr to virObjectIsClass which ends up calling
> virClassIsDerivedFrom using "obj->klass", which wasn't really a klass
> object causing one of those bad things to happen.
> 
> To avoid the future possibility that a non virObject class memory was
> passed to some virObject* API, let's add a "magic_marker" to the base
> virObject class that contains a value "0xFEEDBEEF", then any place in
> the code which would accept or process the base opaque virObjectPtr,
> compare the magic_marker against 0xFEEDBEEF to make sure this is an
> object allocated by this code.
> 
> It is still left up to the caller to handle the failed API calls just
> as it would be if it passed a NULL opaque pointer anyobj.

I don't think that we need to guard this kind of programming error.
We've been able to cope with the current virObject code without hitting
this issue.

Pavel


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

Re: [libvirt] [PATCH go-xml] Add support for Node Device with basic testing.

[Daniel P. Berrange]

On Mon, May 29, 2017 at 03:58:52AM -0400, Vladik Romanovsky wrote:
> ---
>  node_device.go  | 277 
> 
>  node_device_test.go | 111 +
>  2 files changed, 388 insertions(+)
>  create mode 100644 node_device.go
>  create mode 100644 node_device_test.go
> 
> diff --git a/node_device.go b/node_device.go
> new file mode 100644
> index 000..5375d32
> --- /dev/null
> +++ b/node_device.go
> @@ -0,0 +1,277 @@
> +/*
> + * This file is part of the libvirt-go-xml project
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + *
> + * Copyright (C) 2017 Red Hat, Inc.
> + *
> + */
> +
> +package libvirtxml
> +
> +import (
> + "encoding/xml"
> + "fmt"
> +)
> +
> +type NodeDevice struct {
> + Name   string  `xml:"name"`
> + Path   string  `xml:"path,omitempty"`
> + Parent string  `xml:"parent,omitempty"`
> + Driver string  `xml:"driver>name,omitempty"`
> + Capability *CapabilityType `xml:"capability"`
> +}

A single device can have multiple capabilities.

> +
> +type CapabilityType struct {
> + Type interface{} `xml:"type,attr"`
> +}

I'm not convinced about this approach - it differs from what we've
done in other schemas, where we just have a single struct containing
the union of data from all types. The user has no idea what they
should be providing for 'Type' here since its a generic interface
type.

>
> +type IDName struct {
> + ID   string `xml:"id,attr"`
> + Name string `xml:",chardata"`
> +}
> +
> +type PciExpress struct {
> + Links []PciExpressLink `xml:"link"`
> +}
> +
> +type PciExpressLink struct {
> + Validity string  `xml:"validity,attr,omitempty"`
> + Speedfloat64 `xml:"speed,attr,omitempty"`
> + Port int `xml:"port,attr,omitempty"`
> + Widthint `xml:"width,attr,omitempty"`
> +}
> +
> +type IOMMUGroupType struct {
> + Number int `xml:"number,attr"`
> +}
> +
> +type NUMA struct {
> + Node int `xml:"node,attr"`
> +}
> +
> +type PciCapabilityType struct {
> + Domain   int `xml:"domain,omitempty"`
> + Bus  int `xml:"bus,omitempty"`
> + Slot int `xml:"slot,omitempty"`
> + Function int `xml:"function,omitempty"`
> + Product  IDName  `xml:"product,omitempty"`
> + Vendor   IDName  `xml:"vendor,omitempty"`
> + IommuGroup   IOMMUGroupType  `xml:"iommuGroup,omitempty"`
> + Numa NUMA`xml:"numa,omitempty"`
> + PciExpress   PciExpress  `xml:"pci-express,omitempty"`
> + Capabilities []PciCapability `xml:"capability,omitempty"`
> +}
> +
> +type PCIAddress struct {
> + Domain   string `xml:"domain,attr"`
> + Bus  string `xml:"bus,attr"`
> + Slot string `xml:"slot,attr"`
> + Function string `xml:"function,attr"`
> +}
> +
> +type PciCapability struct {

There's alot of inconsistency in capitalization of abbreviations
in this file.  PCI vs Pci,  Numa vs NUMA, IOMMU vs Iiommu. They
should generally always be capitalized.

> + Type string`xml:"type,attr"`
> + Address  []PCIAddress `xml:"address,omitempty"`
> + MaxCount int  `xml:"maxCount,attr,omitempty"`
> +}
> +
> +type SystemHardware struct {
> + Vendor  string `xml:"vendor"`
> + Version string `xml:"version"`
> + Serial  string `xml:"serial"`
> + UUIDstring `xml:"uuid"`
> +}
> +
> +type SystemFirmware struct {
> + Vendor  string `xml:"vendor"`
> + Version string `xml:"version"`
> + ReleaseData string `xml:"release_date"`
> +}
> +
> +type SystemCapabilityType struct {
> + Product  string `xml:"product"`
> + Hardware SystemHardware `xml:"hardware"`
> + Firmware SystemFirmware `xml:"firmware"`
> +}
> +
> +type USBDeviceCapabilityType 

Re: [libvirt] [PATCH v3 9/9] interface: Convert virInterfaceObj to use virObjectLockable

[Bjoern Walk]

John Ferlan  [2017-05-30, 06:43AM -0400]:

[...]
void
-virInterfaceObjFree(virInterfaceObjPtr obj)
+virInterfaceObjEndAPI(virInterfaceObjPtr *obj)


Naming is hard, and I don't have any better suggestion. Just wanted to
say that the name is, maybe, improvable :)


{
-if (!obj)
+if (!*obj)
return;

-virInterfaceDefFree(obj->def);
-virMutexDestroy(>lock);
-VIR_FREE(obj);
+virObjectUnlock(*obj);
+virObjectUnref(*obj);
+*obj = NULL;


I'm a bit conflicted if this is strictly necessary. In what situation
would this bite a consumer if we don't NULL obj? At least in this patch
I can't find any.


}


@@ -140,18 +150,18 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr 
interfaces,
virInterfaceObjPtr obj = interfaces->objs[i];
virInterfaceDefPtr def;

-virInterfaceObjLock(obj);
+virObjectLock(obj);
def = obj->def;
if (STRCASEEQ(def->mac, mac)) {
if (matchct < maxmatches) {
if (VIR_STRDUP(matches[matchct], def->name) < 0) {
-virInterfaceObjUnlock(obj);
+virObjectUnlock(obj);
goto error;
}
matchct++;
}
}
-virInterfaceObjUnlock(obj);
+virObjectUnlock(obj);
}
return matchct;

@@ -173,11 +183,11 @@ virInterfaceObjListFindByName(virInterfaceObjListPtr 
interfaces,
virInterfaceObjPtr obj = interfaces->objs[i];
virInterfaceDefPtr def;

-virInterfaceObjLock(obj);
+virObjectLock(obj);
def = obj->def;
if (STREQ(def->name, name))
-return obj;
-virInterfaceObjUnlock(obj);
+return virObjectRef(obj);
+virObjectUnlock(obj);
}

return NULL;
@@ -190,7 +200,7 @@ virInterfaceObjListFree(virInterfaceObjListPtr interfaces)
size_t i;

for (i = 0; i < interfaces->count; i++)
-virInterfaceObjFree(interfaces->objs[i]);
+virObjectUnref(interfaces->objs[i]);
VIR_FREE(interfaces->objs);
VIR_FREE(interfaces);
}
@@ -227,7 +237,7 @@ virInterfaceObjListClone(virInterfaceObjListPtr interfaces)
VIR_FREE(xml);
if (!(obj = virInterfaceObjListAssignDef(dest, backup)))
goto error;
-virInterfaceObjUnlock(obj); /* locked by virInterfaceObjListAssignDef 
*/
+virInterfaceObjEndAPI();
}

return dest;
@@ -256,13 +266,12 @@ virInterfaceObjListAssignDef(virInterfaceObjListPtr 
interfaces,

if (VIR_APPEND_ELEMENT_COPY(interfaces->objs,
interfaces->count, obj) < 0) {
-virInterfaceObjUnlock(obj);
-virInterfaceObjFree(obj);
+virInterfaceObjEndAPI();
return NULL;
}

obj->def = def;
-return obj;
+return virObjectRef(obj);

}

@@ -273,17 +282,17 @@ virInterfaceObjListRemove(virInterfaceObjListPtr 
interfaces,
{
size_t i;

-virInterfaceObjUnlock(obj);
+virObjectUnlock(obj);
for (i = 0; i < interfaces->count; i++) {
-virInterfaceObjLock(interfaces->objs[i]);
+virObjectLock(interfaces->objs[i]);
if (interfaces->objs[i] == obj) {
-virInterfaceObjUnlock(interfaces->objs[i]);
-virInterfaceObjFree(interfaces->objs[i]);
+virObjectUnlock(interfaces->objs[i]);
+virObjectUnref(interfaces->objs[i]);


Here you could use virInterfaceObjEndAPI if the nulling would be
dropped. Small advantage, I know.


VIR_DELETE_ELEMENT(interfaces->objs, i, interfaces->count);
break;
}
-virInterfaceObjUnlock(interfaces->objs[i]);
+virObjectUnlock(interfaces->objs[i]);
}
}

@@ -297,10 +306,10 @@ virInterfaceObjListNumOfInterfaces(virInterfaceObjListPtr 
interfaces,

for (i = 0; (i < interfaces->count); i++) {
virInterfaceObjPtr obj = interfaces->objs[i];
-virInterfaceObjLock(obj);
+virObjectLock(obj);
if (wantActive == virInterfaceObjIsActive(obj))
ninterfaces++;
-virInterfaceObjUnlock(obj);
+virObjectUnlock(obj);
}

return ninterfaces;
@@ -320,16 +329,16 @@ virInterfaceObjListGetNames(virInterfaceObjListPtr 
interfaces,
virInterfaceObjPtr obj = interfaces->objs[i];
virInterfaceDefPtr def;

-virInterfaceObjLock(obj);
+virObjectLock(obj);
def = obj->def;
if (wantActive == virInterfaceObjIsActive(obj)) {
if (VIR_STRDUP(names[nnames], def->name) < 0) {
-virInterfaceObjUnlock(obj);
+virObjectUnlock(obj);
goto failure;
}
nnames++;
}
-virInterfaceObjUnlock(obj);
+virObjectUnlock(obj);
}

return nnames;
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index 3934e63..2b9e1b2 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -28,6 +28,9 @@ typedef 

Re: [libvirt] [PATCH v3 8/9] interface: Introduce virInterfaceObjNew

[Bjoern Walk]

John Ferlan  [2017-05-30, 06:43AM -0400]:

Create/use a helper to perform the object allocation

Signed-off-by: John Ferlan 
---
src/conf/virinterfaceobj.c | 31 +++
1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 8bd8094..1e3f25c 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -46,6 +46,27 @@ struct _virInterfaceObjList {

/* virInterfaceObj manipulation */

+static virInterfaceObjPtr
+virInterfaceObjNew(void)
+{
+virInterfaceObjPtr obj;
+
+if (VIR_ALLOC(obj) < 0)
+return NULL;
+
+if (virMutexInit(>lock) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("cannot initialize mutex"));
+VIR_FREE(obj);
+return NULL;
+}
+
+virInterfaceObjLock(obj);
+
+return obj;
+}
+
+
void
virInterfaceObjLock(virInterfaceObjPtr obj)
{
@@ -230,18 +251,12 @@ virInterfaceObjListAssignDef(virInterfaceObjListPtr 
interfaces,
return obj;
}

-if (VIR_ALLOC(obj) < 0)
-return NULL;
-if (virMutexInit(>lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("cannot initialize mutex"));
-VIR_FREE(obj);
+if (!(obj = virInterfaceObjNew()))
return NULL;
-}
-virInterfaceObjLock(obj);

if (VIR_APPEND_ELEMENT_COPY(interfaces->objs,
interfaces->count, obj) < 0) {
+virInterfaceObjUnlock(obj);
virInterfaceObjFree(obj);
return NULL;
}
--
2.9.4

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



Looks good to me.

--
IBM Systems
Linux on z Systems & Virtualization Development

IBM Deutschland
Schönaicher Str. 220
71032 Böblingen
Phone: +49 7031 16 1819
E-Mail: bw...@de.ibm.com

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294 


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

[libvirt] [PATCH 8/9] util: Introduce virObjectPoolableDef

[John Ferlan]

Add a new virObjectPoolableHashElement child which will be used to provide
the basis for driver def/newDef elements.

Each virObjectPoolableDef has:

  1. A @recursive argument to denote which type of locks to use.
  2. A required @primaryKey to be used to uniquely identity the object
 by some string value.
  3. An optional @secondaryKey to be used as a secondary means of search
 for the object by some string value.
  4. A required @def and @defFreeFunc. The @def will be consumed by the
 object and when disposed the free function will be called.

The _virObjectPoolableDef has an additional @newDef element to store
the "next" boot configuration for consumers that support the functionality.

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms |  2 ++
 src/util/virobject.c | 85 +++-
 src/util/virobject.h | 25 ++
 3 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fea0be7..4fad7c8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2258,6 +2258,7 @@ virNumaSetupMemoryPolicy;
 # util/virobject.h
 virClassForObject;
 virClassForObjectLockable;
+virClassForObjectPoolableDef;
 virClassForObjectPoolableHashElement;
 virClassIsDerivedFrom;
 virClassName;
@@ -2271,6 +2272,7 @@ virObjectLock;
 virObjectLockableNew;
 virObjectLockableRecursiveNew;
 virObjectNew;
+virObjectPoolableDefNew;
 virObjectPoolableHashElementGetPrimaryKey;
 virObjectPoolableHashElementGetSecondaryKey;
 virObjectPoolableHashElementNew;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 74299f8..d1fc3f0 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -62,9 +62,11 @@ struct _virClass {
 static virClassPtr virObjectClass;
 static virClassPtr virObjectLockableClass;
 static virClassPtr virObjectPoolableHashElementClass;
+static virClassPtr virObjectPoolableDefClass;
 
 static void virObjectLockableDispose(void *anyobj);
 static void virObjectPoolableHashElementDispose(void *anyobj);
+static void virObjectPoolableDefDispose(void *anyobj);
 
 static int
 virObjectOnceInit(void)
@@ -88,6 +90,13 @@ virObjectOnceInit(void)
   virObjectPoolableHashElementDispose)))
 return -1;
 
+if (!(virObjectPoolableDefClass =
+  virClassNew(virObjectPoolableHashElementClass,
+  "virObjectPoolableDef",
+  sizeof(virObjectPoolableDef),
+  virObjectPoolableDefDispose)))
+return -1;
+
 return 0;
 }
 
@@ -142,6 +151,23 @@ virClassForObjectPoolableHashElement(void)
 
 
 /**
+ * virClassForObjectPoolableDef:
+ *
+ * Returns the class instance for the virObjectPoolableDef type
+ */
+virClassPtr
+virClassForObjectPoolableDef(void)
+{
+if (virObjectInitialize() < 0)
+return NULL;
+
+VIR_DEBUG("virObjectPoolableDefClass=%p",
+  virObjectPoolableDefClass);
+return virObjectPoolableDefClass;
+}
+
+
+/**
  * virClassNew:
  * @parent: the parent class
  * @name: the class name
@@ -365,6 +391,62 @@ virObjectPoolableHashElementDispose(void *anyobj)
 
 
 /**
+ * virObjectPoolableDefNew:
+ * @klass: the klass to check
+ * @recursive: boolean to dictate which Lockable object to use
+ * @primaryKey: primary key (required)
+ * @secondaryKey: secondary key
+ * @def: XML definition (required)
+ * @defFreeFunc: Free function for @def and @newDef (required)
+ *
+ * Create a new poolable def object for storing "common" domain defs.
+ *
+ * Returns: New object on success, NULL on failure w/ error message set
+ */
+void *
+virObjectPoolableDefNew(virClassPtr klass,
+bool recursive,
+const char *primaryKey,
+const char *secondaryKey,
+void *def,
+virFreeCallback defFreeFunc)
+{
+virObjectPoolableDefPtr obj;
+
+if (!virClassIsDerivedFrom(klass, virClassForObjectPoolableDef())) {
+virReportInvalidArg(klass,
+_("Class %s must derive from "
+  "virObjectPoolableDef"),
+virClassName(klass));
+return NULL;
+}
+
+if (!(obj = virObjectPoolableHashElementNew(klass, recursive,
+primaryKey, secondaryKey)))
+return NULL;
+
+obj->def = def;
+obj->defFreeFunc = defFreeFunc;
+
+VIR_DEBUG("obj=%p, def=%p ff=%p", obj, obj->def, obj->defFreeFunc);
+
+return obj;
+}
+
+
+static void
+virObjectPoolableDefDispose(void *anyobj)
+{
+virObjectPoolableDefPtr obj = anyobj;
+
+VIR_DEBUG("dispose obj=%p", obj);
+
+(obj->defFreeFunc)(obj->def);
+(obj->defFreeFunc)(obj->newDef);
+}
+
+
+/**
  * virObjectUnref:
  * @anyobj: any instance of virObjectPtr
  *
@@ -432,7 +514,8 @@ static virObjectLockablePtr
 virObjectGetLockableObj(void *anyobj)
 {
 

[libvirt] [PATCH 1/9] util: Formatting cleanups to virobject API

[John Ferlan]

Alter to use more recent formatting guidelines

Signed-off-by: John Ferlan 
---
 src/util/virobject.c | 64 ++--
 src/util/virobject.h | 59 
 2 files changed, 82 insertions(+), 41 deletions(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index 51876b9..792685b 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -52,7 +52,8 @@ static virClassPtr virObjectLockableClass;
 
 static void virObjectLockableDispose(void *anyobj);
 
-static int virObjectOnceInit(void)
+static int
+virObjectOnceInit(void)
 {
 if (!(virObjectClass = virClassNew(NULL,
"virObject",
@@ -77,7 +78,8 @@ VIR_ONCE_GLOBAL_INIT(virObject);
  *
  * Returns the class instance for the base virObject type
  */
-virClassPtr virClassForObject(void)
+virClassPtr
+virClassForObject(void)
 {
 if (virObjectInitialize() < 0)
 return NULL;
@@ -91,7 +93,8 @@ virClassPtr virClassForObject(void)
  *
  * Returns the class instance for the virObjectLockable type
  */
-virClassPtr virClassForObjectLockable(void)
+virClassPtr
+virClassForObjectLockable(void)
 {
 if (virObjectInitialize() < 0)
 return NULL;
@@ -116,10 +119,11 @@ virClassPtr virClassForObjectLockable(void)
  *
  * Returns a new class instance
  */
-virClassPtr virClassNew(virClassPtr parent,
-const char *name,
-size_t objectSize,
-virObjectDisposeCallback dispose)
+virClassPtr
+virClassNew(virClassPtr parent,
+const char *name,
+size_t objectSize,
+virObjectDisposeCallback dispose)
 {
 virClassPtr klass;
 
@@ -162,8 +166,9 @@ virClassPtr virClassNew(virClassPtr parent,
  *
  * Return true if @klass is derived from @parent, false otherwise
  */
-bool virClassIsDerivedFrom(virClassPtr klass,
-   virClassPtr parent)
+bool
+virClassIsDerivedFrom(virClassPtr klass,
+  virClassPtr parent)
 {
 while (klass) {
 if (klass->magic == parent->magic)
@@ -186,7 +191,8 @@ bool virClassIsDerivedFrom(virClassPtr klass,
  *
  * Returns the new object
  */
-void *virObjectNew(virClassPtr klass)
+void *
+virObjectNew(virClassPtr klass)
 {
 virObjectPtr obj = NULL;
 
@@ -205,7 +211,8 @@ void *virObjectNew(virClassPtr klass)
 }
 
 
-void *virObjectLockableNew(virClassPtr klass)
+void *
+virObjectLockableNew(virClassPtr klass)
 {
 virObjectLockablePtr obj;
 
@@ -230,13 +237,15 @@ void *virObjectLockableNew(virClassPtr klass)
 }
 
 
-static void virObjectLockableDispose(void *anyobj)
+static void
+virObjectLockableDispose(void *anyobj)
 {
 virObjectLockablePtr obj = anyobj;
 
 virMutexDestroy(>lock);
 }
 
+
 /**
  * virObjectUnref:
  * @anyobj: any instance of virObjectPtr
@@ -248,7 +257,8 @@ static void virObjectLockableDispose(void *anyobj)
  * Returns true if the remaining reference count is
  * non-zero, false if the object was disposed of
  */
-bool virObjectUnref(void *anyobj)
+bool
+virObjectUnref(void *anyobj)
 {
 virObjectPtr obj = anyobj;
 
@@ -286,7 +296,8 @@ bool virObjectUnref(void *anyobj)
  *
  * Returns @anyobj
  */
-void *virObjectRef(void *anyobj)
+void *
+virObjectRef(void *anyobj)
 {
 virObjectPtr obj = anyobj;
 
@@ -310,7 +321,8 @@ void *virObjectRef(void *anyobj)
  * The object must be unlocked before releasing this
  * reference.
  */
-void virObjectLock(void *anyobj)
+void
+virObjectLock(void *anyobj)
 {
 virObjectLockablePtr obj = anyobj;
 
@@ -331,7 +343,8 @@ void virObjectLock(void *anyobj)
  * Release a lock on @anyobj. The lock must have been
  * acquired by virObjectLock.
  */
-void virObjectUnlock(void *anyobj)
+void
+virObjectUnlock(void *anyobj)
 {
 virObjectLockablePtr obj = anyobj;
 
@@ -355,8 +368,9 @@ void virObjectUnlock(void *anyobj)
  *
  * Returns true if @anyobj is an instance of @klass
  */
-bool virObjectIsClass(void *anyobj,
-  virClassPtr klass)
+bool
+virObjectIsClass(void *anyobj,
+ virClassPtr klass)
 {
 virObjectPtr obj = anyobj;
 if (!obj)
@@ -372,7 +386,8 @@ bool virObjectIsClass(void *anyobj,
  *
  * Returns the name of @klass
  */
-const char *virClassName(virClassPtr klass)
+const char *
+virClassName(virClassPtr klass)
 {
 return klass->name;
 }
@@ -401,7 +416,9 @@ void virObjectFreeCallback(void *opaque)
  * but with the signature matching the virHashDataFree
  * typedef.
  */
-void virObjectFreeHashData(void *opaque, const void *name ATTRIBUTE_UNUSED)
+void
+virObjectFreeHashData(void *opaque,
+  const void *name ATTRIBUTE_UNUSED)
 {
 virObjectUnref(opaque);
 }
@@ -413,7 +430,8 @@ void virObjectFreeHashData(void *opaque, const void *name 
ATTRIBUTE_UNUSED)
  *
  * Unrefs all members of @list and frees the list itself.
  */
-void virObjectListFree(void *list)
+void
+virObjectListFree(void 

[libvirt] [PATCH 6/9] util: Introduce virObjectPoolableHashElement

[John Ferlan]

Add a new virObjectLockable child which will be used to more generically
describe driver objects. Eventually these objects will be placed into a
more generic hash table object which will take care of object mgmt functions.

Each virObjectPoolableHashElement will have a primaryKey (required) and a
secondaryKey (optional) which can be used to insert the same object into
two hash tables for faster lookups.

The recursive parameter controls which type of lock is used.

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms |  2 ++
 src/util/virobject.c | 82 +++-
 src/util/virobject.h | 18 +++
 3 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9693dc4..eca1980 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2258,6 +2258,7 @@ virNumaSetupMemoryPolicy;
 # util/virobject.h
 virClassForObject;
 virClassForObjectLockable;
+virClassForObjectPoolableHashElement;
 virClassIsDerivedFrom;
 virClassName;
 virClassNew;
@@ -2270,6 +2271,7 @@ virObjectLock;
 virObjectLockableNew;
 virObjectLockableRecursiveNew;
 virObjectNew;
+virObjectPoolableHashElementNew;
 virObjectRef;
 virObjectUnlock;
 virObjectUnref;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index b1fabd7..25dbb8f 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -61,8 +61,10 @@ struct _virClass {
 
 static virClassPtr virObjectClass;
 static virClassPtr virObjectLockableClass;
+static virClassPtr virObjectPoolableHashElementClass;
 
 static void virObjectLockableDispose(void *anyobj);
+static void virObjectPoolableHashElementDispose(void *anyobj);
 
 static int
 virObjectOnceInit(void)
@@ -79,6 +81,13 @@ virObjectOnceInit(void)
virObjectLockableDispose)))
 return -1;
 
+if (!(virObjectPoolableHashElementClass =
+  virClassNew(virObjectLockableClass,
+  "virObjectPoolableHashElement",
+  sizeof(virObjectPoolableHashElement),
+  virObjectPoolableHashElementDispose)))
+return -1;
+
 return 0;
 }
 
@@ -116,6 +125,23 @@ virClassForObjectLockable(void)
 
 
 /**
+ * virClassForObjectPoolableHashElement:
+ *
+ * Returns the class instance for the virObjectPoolableHashElement type
+ */
+virClassPtr
+virClassForObjectPoolableHashElement(void)
+{
+if (virObjectInitialize() < 0)
+return NULL;
+
+VIR_DEBUG("virObjectPoolableHashElementClass=%p",
+  virObjectPoolableHashElementClass);
+return virObjectPoolableHashElementClass;
+}
+
+
+/**
  * virClassNew:
  * @parent: the parent class
  * @name: the class name
@@ -285,6 +311,59 @@ virObjectLockableDispose(void *anyobj)
 }
 
 
+void *
+virObjectPoolableHashElementNew(virClassPtr klass,
+bool recursive,
+const char *primaryKey,
+const char *secondaryKey)
+{
+virObjectPoolableHashElementPtr obj;
+
+if (!virClassIsDerivedFrom(klass, virClassForObjectPoolableHashElement())) 
{
+virReportInvalidArg(klass,
+_("Class %s must derive from "
+  "virObjectPoolableHashElement"),
+virClassName(klass));
+return NULL;
+}
+
+if (!recursive) {
+if (!(obj = virObjectLockableNew(klass)))
+return NULL;
+} else {
+if (!(obj = virObjectLockableRecursiveNew(klass)))
+return NULL;
+}
+
+if (VIR_STRDUP(obj->primaryKey, primaryKey) < 0)
+goto error;
+
+if (secondaryKey && VIR_STRDUP(obj->secondaryKey, secondaryKey) < 0)
+goto error;
+
+VIR_DEBUG("obj=%p, primary=%s secondary=%s",
+  obj, obj->primaryKey, NULLSTR(obj->secondaryKey));
+
+return obj;
+
+ error:
+virObjectUnref(obj);
+return NULL;
+}
+
+
+static void
+virObjectPoolableHashElementDispose(void *anyobj)
+{
+virObjectPoolableHashElementPtr obj = anyobj;
+
+VIR_DEBUG("dispose obj=%p", obj);
+
+VIR_FREE(obj->primaryKey);
+VIR_FREE(obj->secondaryKey);
+}
+
+
 /**
  * virObjectUnref:
  * @anyobj: any instance of virObjectPtr
@@ -352,7 +431,8 @@ virObjectRef(void *anyobj)
 static virObjectLockablePtr
 virObjectGetLockableObj(void *anyobj)
 {
-if (virObjectIsClass(anyobj, virObjectLockableClass))
+if (virObjectIsClass(anyobj, virObjectLockableClass) ||
+virObjectIsClass(anyobj, virObjectPoolableHashElementClass))
 return anyobj;
 
 VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectLockableClass);
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 7df9b47..0377cd5 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -34,6 +34,9 @@ typedef virObject *virObjectPtr;
 typedef struct _virObjectLockable virObjectLockable;
 typedef virObjectLockable 

[libvirt] [PATCH 4/9] util: Add magic_marker for all virObjects

[John Ferlan]

The virObject logic "assumes" that whatever is passed to its API's
would be some sort of virObjectPtr; however, if it is not then some
really bad things can happen.

So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock},
and virObjectIsClass and the virObject and virObjectLockable class
consumers have been well behaved and code well tested. Soon there will
be more consumers and one such consumer tripped over this during testing
by passing a virHashTablePtr to virObjectIsClass which ends up calling
virClassIsDerivedFrom using "obj->klass", which wasn't really a klass
object causing one of those bad things to happen.

To avoid the future possibility that a non virObject class memory was
passed to some virObject* API, let's add a "magic_marker" to the base
virObject class that contains a value "0xFEEDBEEF", then any place in
the code which would accept or process the base opaque virObjectPtr,
compare the magic_marker against 0xFEEDBEEF to make sure this is an
object allocated by this code.

It is still left up to the caller to handle the failed API calls just
as it would be if it passed a NULL opaque pointer anyobj.

Signed-off-by: John Ferlan 
---
 src/util/virobject.c | 12 
 src/util/virobject.h |  1 +
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index 9f5f187..a1934941 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -47,10 +47,12 @@ struct _virClass {
 virObjectDisposeCallback dispose;
 };
 
+#define VIR_OBJECT_NOTVALID(obj) (!obj || obj->magic_marker != 0xFEEDBEEF)
+
 #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass)\
 do {\
 virObjectPtr obj = anyobj;  \
-if (!obj)   \
+if (VIR_OBJECT_NOTVALID(obj))   \
 VIR_WARN("Object %p is not a virObject class instance", anyobj);\
 else\
 VIR_WARN("Object %p (%s) is not a %s instance", \
@@ -212,6 +214,7 @@ virObjectNew(virClassPtr klass)
 return NULL;
 
 obj->u.s.magic = klass->magic;
+obj->magic_marker = 0xFEEDBEEF;
 obj->klass = klass;
 virAtomicIntSet(>u.s.refs, 1);
 
@@ -272,7 +275,7 @@ virObjectUnref(void *anyobj)
 {
 virObjectPtr obj = anyobj;
 
-if (!obj)
+if (VIR_OBJECT_NOTVALID(obj))
 return false;
 
 bool lastRef = virAtomicIntDecAndTest(>u.s.refs);
@@ -289,6 +292,7 @@ virObjectUnref(void *anyobj)
 /* Clear & poison object */
 memset(obj, 0, obj->klass->objectSize);
 obj->u.s.magic = 0xDEADBEEF;
+obj->magic_marker = 0xDEADBEEF;
 obj->klass = (void*)0xDEADBEEF;
 VIR_FREE(obj);
 }
@@ -311,7 +315,7 @@ virObjectRef(void *anyobj)
 {
 virObjectPtr obj = anyobj;
 
-if (!obj)
+if (VIR_OBJECT_NOTVALID(obj))
 return NULL;
 virAtomicIntInc(>u.s.refs);
 PROBE(OBJECT_REF, "obj=%p", obj);
@@ -389,7 +393,7 @@ virObjectIsClass(void *anyobj,
  virClassPtr klass)
 {
 virObjectPtr obj = anyobj;
-if (!obj)
+if (VIR_OBJECT_NOTVALID(obj))
 return false;
 
 return virClassIsDerivedFrom(obj->klass, klass);
diff --git a/src/util/virobject.h b/src/util/virobject.h
index f4c292b..89f8050 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -51,6 +51,7 @@ struct _virObject {
 int refs;
 } s;
 } u;
+unsigned int magic_marker;
 virClassPtr klass;
 };
 
-- 
2.9.4

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

[libvirt] [PATCH 2/9] util: Introduce virObjectGetLockableObj

[John Ferlan]

Split out the object fetch in virObject{Lock|Unlock} into a helper

Signed-off-by: John Ferlan 
---
 src/util/virobject.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index 792685b..34805d3 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -309,6 +309,22 @@ virObjectRef(void *anyobj)
 }
 
 
+static virObjectLockablePtr
+virObjectGetLockableObj(void *anyobj)
+{
+virObjectPtr obj;
+
+if (virObjectIsClass(anyobj, virObjectLockableClass))
+return anyobj;
+
+obj = anyobj;
+VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
+  anyobj, obj ? obj->klass->name : "(unknown)");
+
+return NULL;
+}
+
+
 /**
  * virObjectLock:
  * @anyobj: any instance of virObjectLockablePtr
@@ -324,13 +340,10 @@ virObjectRef(void *anyobj)
 void
 virObjectLock(void *anyobj)
 {
-virObjectLockablePtr obj = anyobj;
+virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
 
-if (!virObjectIsClass(obj, virObjectLockableClass)) {
-VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
- obj, obj ? obj->parent.klass->name : "(unknown)");
+if (!obj)
 return;
-}
 
 virMutexLock(>lock);
 }
@@ -346,13 +359,10 @@ virObjectLock(void *anyobj)
 void
 virObjectUnlock(void *anyobj)
 {
-virObjectLockablePtr obj = anyobj;
+virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
 
-if (!virObjectIsClass(obj, virObjectLockableClass)) {
-VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
- obj, obj ? obj->parent.klass->name : "(unknown)");
+if (!obj)
 return;
-}
 
 virMutexUnlock(>lock);
 }
-- 
2.9.4

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

[libvirt] [PATCH 9/9] util: Add virObjectPoolableDef* accessor API's

[John Ferlan]

Add new API's for object management:

virObjectPoolableDefGetDef

- Just return the obj->def from the object.

virObjectPoolableDefSetDef

- If the new @def is non-NULL, replace the previous obj->def with
  the new @def argument calling the obj->defFreeFunc on the previous
  obj->def.  If the @def is NULL, then just clear the obj->def
  leaving the caller to handle removal of the previous obj->def.

virObjectPoolableDefGetNewDef

- Just return the obj->newDef from the object.

virObjectPoolableDefSetNewDef

- If the new @newDef is non-NULL, replace the previous obj->newDef
  with the new @newDef argument calling the obj->defFreeFunc on
  the previous obj->newDef.  If the @newDef is NULL, then just clear
  the obj->newDef leaving the caller to handle removal of the
  previous obj->newDef.

virObjectPoolableDefSwapNewDef

   - Manage swapping the obj->newDef into obj->def by first calling
 the obj->defFreeFunc on the current obj->def and then stealing
 the obj->newDef into obj->def.

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms |   5 ++
 src/util/virobject.c | 143 +++
 src/util/virobject.h |  17 ++
 3 files changed, 165 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4fad7c8..f6e40c7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2272,7 +2272,12 @@ virObjectLock;
 virObjectLockableNew;
 virObjectLockableRecursiveNew;
 virObjectNew;
+virObjectPoolableDefGetDef;
+virObjectPoolableDefGetNewDef;
 virObjectPoolableDefNew;
+virObjectPoolableDefSetDef;
+virObjectPoolableDefSetNewDef;
+virObjectPoolableDefStealNewDef;
 virObjectPoolableHashElementGetPrimaryKey;
 virObjectPoolableHashElementGetSecondaryKey;
 virObjectPoolableHashElementNew;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index d1fc3f0..ef253b6 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -536,6 +536,18 @@ virObjectGetPoolableHashElementObj(void *anyobj)
 }
 
 
+static virObjectPoolableDefPtr
+virObjectGetPoolableDefObj(void *anyobj)
+{
+if (virObjectIsClass(anyobj, virObjectPoolableDefClass))
+return anyobj;
+
+VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectPoolableDefClass);
+
+return NULL;
+}
+
+
 /**
  * virObjectLock:
  * @anyobj: any instance of virObjectLockablePtr
@@ -725,3 +737,134 @@ virObjectPoolableHashElementGetSecondaryKey(void *anyobj)
 
 return obj->secondaryKey;
 }
+
+
+/**
+ * virObjectPoolableDefGetDef
+ * @anyobj: Pointer to a locked PoolableDef object
+ *
+ * Returns: Pointer to the object @def or NULL on failure
+ */
+void *
+virObjectPoolableDefGetDef(void *anyobj)
+{
+virObjectPoolableDefPtr obj = virObjectGetPoolableDefObj(anyobj);
+
+if (!obj)
+return NULL;
+
+return obj->def;
+}
+
+
+/**
+ * virObjectPoolableDefSetDef
+ * @anyobj: Pointer to a locked PoolableDef object
+ * @def: Opaque object definition to replace in the object
+ *
+ * Set the @def of the object - this may be NULL if clearing the @def
+ * from the object in which case it is up to the caller to handle managing
+ * the previous obj->def. If @def is not NULL, this includes a call to the
+ * defFreeFunc prior to the set.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virObjectPoolableDefSetDef(void *anyobj,
+   void *def)
+{
+virObjectPoolableDefPtr obj = virObjectGetPoolableDefObj(anyobj);
+
+if (!obj)
+return -1;
+
+if (def) {
+obj->defFreeFunc(obj->def);
+obj->def = def;
+} else {
+obj->def = NULL;
+}
+
+return 0;
+}
+
+
+/**
+ * virObjectPoolableDefGetNewDef
+ * @anyobj: Pointer to a locked PoolableDef object
+ *
+ * Returns: Pointer to the object 'newDef' or NULL on failure
+ */
+void *
+virObjectPoolableDefGetNewDef(void *anyobj)
+{
+virObjectPoolableDefPtr obj = virObjectGetPoolableDefObj(anyobj);
+
+if (!obj)
+return NULL;
+
+return obj->newDef;
+}
+
+
+/**
+ * virObjectPoolableDefSetNewDef
+ * @anyobj: Pointer to a locked PoolableDef object
+ * @newDef: Opaque 'newDef' to replace in the object, may be NULL
+ *
+ * Set the 'newDef' of the object - this may be NULL if clearing the newDef
+ * from the object in which case it is up to the caller to handle managing
+ * the previous newDef. If @newDef is not NULL, this includes a call to the
+ * defFreeFunc prior to the set.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virObjectPoolableDefSetNewDef(void *anyobj,
+  void *newDef)
+{
+virObjectPoolableDefPtr obj = virObjectGetPoolableDefObj(anyobj);
+
+if (!obj)
+return -1;
+
+if (newDef) {
+obj->defFreeFunc(obj->newDef);
+obj->newDef = newDef;
+} else {
+obj->newDef = NULL;
+}
+
+return 0;
+}
+
+
+/**
+ * 

[libvirt] [PATCH 5/9] util: Introduce virObjectLockableRecursiveNew

[John Ferlan]

Introduce a recursive option for the lockable object which calls
virMutexInitRecursive instead of virMutexInit.

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms |  1 +
 src/util/virobject.c | 26 ++
 src/util/virobject.h |  4 
 3 files changed, 31 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 429b095..9693dc4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2268,6 +2268,7 @@ virObjectListFree;
 virObjectListFreeCount;
 virObjectLock;
 virObjectLockableNew;
+virObjectLockableRecursiveNew;
 virObjectNew;
 virObjectRef;
 virObjectUnlock;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index a1934941..b1fabd7 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -250,6 +250,32 @@ virObjectLockableNew(virClassPtr klass)
 }
 
 
+void *
+virObjectLockableRecursiveNew(virClassPtr klass)
+{
+virObjectLockablePtr obj;
+
+if (!virClassIsDerivedFrom(klass, virClassForObjectLockable())) {
+virReportInvalidArg(klass,
+_("Class %s must derive from virObjectLockable"),
+virClassName(klass));
+return NULL;
+}
+
+if (!(obj = virObjectNew(klass)))
+return NULL;
+
+if (virMutexInitRecursive(>lock) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Unable to initialize recursive mutex"));
+virObjectUnref(obj);
+return NULL;
+}
+
+return obj;
+}
+
+
 static void
 virObjectLockableDispose(void *anyobj)
 {
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 89f8050..7df9b47 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -109,6 +109,10 @@ void *
 virObjectLockableNew(virClassPtr klass)
 ATTRIBUTE_NONNULL(1);
 
+void *
+virObjectLockableRecursiveNew(virClassPtr klass)
+ATTRIBUTE_NONNULL(1);
+
 void
 virObjectLock(void *lockableobj)
 ATTRIBUTE_NONNULL(1);
-- 
2.9.4

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

[libvirt] [PATCH 7/9] util: Add virObjectPoolableHashElementGet*Key

[John Ferlan]

Add a pair of accessor API's:

   virObjectPoolableHashElementGetPrimaryKey
   virObjectPoolableHashElementGetSecondaryKey

which will return the requested key from the object to the caller.

It is up to the caller to check the returned key and error if the
return value is NULL.

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms |  2 ++
 src/util/virobject.c | 50 
 src/util/virobject.h |  6 ++
 3 files changed, 58 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index eca1980..fea0be7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2271,6 +2271,8 @@ virObjectLock;
 virObjectLockableNew;
 virObjectLockableRecursiveNew;
 virObjectNew;
+virObjectPoolableHashElementGetPrimaryKey;
+virObjectPoolableHashElementGetSecondaryKey;
 virObjectPoolableHashElementNew;
 virObjectRef;
 virObjectUnlock;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 25dbb8f..74299f8 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -441,6 +441,18 @@ virObjectGetLockableObj(void *anyobj)
 }
 
 
+static virObjectPoolableHashElementPtr
+virObjectGetPoolableHashElementObj(void *anyobj)
+{
+if (virObjectIsClass(anyobj, virObjectPoolableHashElementClass))
+return anyobj;
+
+VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectPoolableHashElementClass);
+
+return NULL;
+}
+
+
 /**
  * virObjectLock:
  * @anyobj: any instance of virObjectLockablePtr
@@ -592,3 +604,41 @@ virObjectListFreeCount(void *list,
 
 VIR_FREE(list);
 }
+
+
+/**
+ * virObjectPoolableHashElementGetPrimaryKey
+ * @anyobj: Pointer to a PoolableHashElement object
+ *
+ * Returns: Pointer to the primaryKey or NULL on failure
+ */
+const char *
+virObjectPoolableHashElementGetPrimaryKey(void *anyobj)
+{
+virObjectPoolableHashElementPtr obj =
+virObjectGetPoolableHashElementObj(anyobj);
+
+if (!obj)
+return NULL;
+
+return obj->primaryKey;
+}
+
+
+/**
+ * virObjectPoolableHashElementGetSecondaryKey
+ * @anyobj: Pointer to a PoolableHashElement object
+ *
+ * Returns: Pointer to the secondaryKey which may be NULL
+ */
+const char *
+virObjectPoolableHashElementGetSecondaryKey(void *anyobj)
+{
+virObjectPoolableHashElementPtr obj =
+virObjectGetPoolableHashElementObj(anyobj);
+
+if (!obj)
+return NULL;
+
+return obj->secondaryKey;
+}
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 0377cd5..4706502 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -146,4 +146,10 @@ void
 virObjectListFreeCount(void *list,
size_t count);
 
+const char *
+virObjectPoolableHashElementGetPrimaryKey(void *anyobj);
+
+const char *
+virObjectPoolableHashElementGetSecondaryKey(void *anyobj);
+
 #endif /* __VIR_OBJECT_H */
-- 
2.9.4

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

[libvirt] [PATCH 0/9] virObject adjustments for common object

[John Ferlan]

Originally posted in part as an RFC:

https://www.redhat.com/archives/libvir-list/2017-April/msg00321.html

Obviously not a 3.4.0 discussion, but I'm trying to get ahead of the curve
for 3.5.0 and posting this now as I'm "close enough" to at least convert
secrets, nwfilters, nodedevs, and interfaces to use a more common object
methodolgy to add/store, fetch/find, search, list, etc. the objects in
something other than a linear forward linked list. Converting the storage
pool, storage volume, and network is taking longer than hoped since there
are so many patches to get from pointA to pointB. So I'll focus on the
4 and work through getting the complete model published/adopted as those
have been in my branches for a while now.

Still these patches get as far as the creation of the common object which
is a start. The first 4 patches are essentially setup. Patch 5 is new
and covers the need for nwfilters (at least temporarily until self locking
list mgmt is possible). Patches 6-7 were more or less what patch 7-8 were
in the RFC. Patches 8-9 are essentially the driver/common object extension.
Patches 5-6 from the RFC still exist in my branches and would be the next
logical step. It was a conscious decision to just use the "default" string
comparison for hash table key. That key could be a UUID, but since the
UUID can easily be converted from unsigned to signed - I believe it would
be far simpler to keep that mechanism rather than complexity introduced
by having "different" key search algorithms when the key isn't a char *.

John Ferlan (9):
  util: Formatting cleanups to virobject API
  util: Introduce virObjectGetLockableObj
  util: Generate a common internal only error print
  util: Add magic_marker for all virObjects
  util: Introduce virObjectLockableRecursiveNew
  util: Introduce virObjectPoolableHashElement
  util: Add virObjectPoolableHashElementGet*Key
  util: Introduce virObjectPoolableDef
  util: Add virObjectPoolableDef* accessor API's

 src/libvirt_private.syms |  12 ++
 src/util/virobject.c | 492 +++
 src/util/virobject.h | 130 +++--
 3 files changed, 580 insertions(+), 54 deletions(-)

-- 
2.9.4

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

[libvirt] [PATCH] qemu: hotplug: Release address properly when redirected device attach failure

[Shivaprasad G Bhat]

The virDomainUSBAddressEnsure returns 0 or -1 and checking for 1 is wrong.
Fix it.

Signed-off-by: Shivaprasad G Bhat 
---
 src/qemu/qemu_hotplug.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4a7d997..f339148 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1707,7 +1707,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
virDomainRedirdevDefPtr redirdev)
 {
 int ret = -1;
-int rc;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virDomainDefPtr def = vm->def;
 char *charAlias = NULL;
@@ -1724,10 +1723,9 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
 if (!(charAlias = qemuAliasChardevFromDevAlias(redirdev->info.alias)))
 goto cleanup;
 
-if ((rc = virDomainUSBAddressEnsure(priv->usbaddrs, >info)) < 0)
+if ((virDomainUSBAddressEnsure(priv->usbaddrs, >info)) < 0)
 goto cleanup;
-if (rc == 1)
-need_release = true;
+need_release = true;
 
 if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps)))
 goto cleanup;

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

Re: [libvirt] [PATCH] qemu: Don't error out if allocation info can't be queried

[John Ferlan]

On 05/29/2017 11:04 AM, Peter Krempa wrote:
> qemuDomainGetBlockInfo would error out if qemu did not report
> 'wr_highest_offset'. This usually does not happen, but can happen
> briefly during active layer block commit. There's no need to report the
> error, we can simply report that the disk is fully alocated at that

s/alocated/allocated

> point.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1452045
> ---
>  src/qemu/qemu_driver.c | 8 
>  1 file changed, 8 deletions(-)
> 

I am curious why this isn't a QEMU bug?

Secondarily given what I read in the bz, if the qemuDomainGetBlockInfo
thread does continue it will display the physical (which is fine), but
once the qemu command can succeed again and wr_highest_offset is valid
again - will it be less than the physical now?  IOW: Is there a "period"
that an incorrect value would be displayed because of the failure to
return the value and could that be misconstrued some how?  Should that
be documented in the block commit API description?

Side thought - do we perhaps want to add a VIR_DEBUG msg about the
missing data?

Reviewed-by: John Ferlan 

John

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 67f54282a..f6b352b56 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11541,14 +11541,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
>  }
> 
>  if (!entry->wr_highest_offset_valid) {
> -if (virStorageSourceGetActualType(disk->src) == 
> VIR_STORAGE_TYPE_BLOCK &&
> -disk->src->format != VIR_STORAGE_FILE_RAW) {
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> -   _("failed to query the maximum written offset of "
> - "block device '%s'"), disk->dst);
> -goto endjob;
> -}
> -
>  info->allocation = entry->physical;
>  } else {
>  if (virStorageSourceGetActualType(disk->src) == 
> VIR_STORAGE_TYPE_FILE &&
> 

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

[libvirt] [GSoC] Project of libvirt/qemu fuzzing

[Dan]

Dear all,

The project of qemu command line fuzzing has been accepted as a GSoC
project [1] [2]. As a student participating Google Summer of Code
activity, I am extremely exitited to get started today on May 30th,
2017. During the past months, I have received tremendous guidance
from my mentors as well as many other contributors on the
mailinglist. I look forward to contributing to the community and
learning a lot over the summer. Any advice, comment, feedback,
suggestion to my emails/commit, would always be highly appreciated
and more than welcome.

Thank you all for your time,

Daniel Liu

[1]. https://summerofcode.withgoogle.com/projects/#5088017038442496
[2]. 
https://wiki.libvirt.org/page/Google_Summer_of_Code_Ideas#QEMU_command_line_generator_XML_fuzzing

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

Re: [libvirt] [libvirt-sandbox PATCH] Drop library/ from template name and image path

[Daniel P. Berrange]

On Sat, May 27, 2017 at 06:29:32PM +0200, Guido Günther wrote:
> If one pastes from the output of virt-sansbox-image
> 
>   $ virt-sandbox-image list
>   docker:/library/ubuntu?tag=17.04
>   docker:/library/debian?tag=latest
> 
> verbatim
> 
>   $ virt-sandbox-image run -c qemu:///session 
> docker:/library/debian?tag=latest
> 
> This fails like
> 
>   /home//.local/share/libvirt/images/library/debian:qbeilwxard.qcow2: 
> Could not create file: No such file or directory
>   Unable to start sandbox: Failed to create domain: XML error: name 
> library/debian:qbeilwxard cannot contain '/'
>   /usr/bin/virt-sandbox-image: [Errno 2] No such file or directory: 
> '/home//.local/share/libvirt/images/library/debian:qbeilwxard.qcow2'
> 
> so strip of any leading components.
> ---
> This might not be the correct fix but I hope you get the idea.

We need to replace '/' with "_" - in fact I'd suggest we replace any char
exception a-Z, 0-9 and -.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH 2/2] fdstream: Report error from the I/O thread

[Michal Privoznik]

Problem with our error reporting is that the error object is a
thread local variable. That means if there's an error reported
within the I/O thread it gets logged and everything, but later
when the event loop aborts the stream it doesn't see the original
error. So we are left with some generic error. We can do better
if we copy the error message between the threads.

Signed-off-by: Michal Privoznik 
---
 daemon/stream.c| 18 --
 src/util/virfdstream.c |  9 ++---
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/daemon/stream.c b/daemon/stream.c
index 1d5b50ad7..5077ac8b0 100644
--- a/daemon/stream.c
+++ b/daemon/stream.c
@@ -231,17 +231,23 @@ daemonStreamEvent(virStreamPtr st, int events, void 
*opaque)
 int ret;
 virNetMessagePtr msg;
 virNetMessageError rerr;
+virErrorPtr origErr = virSaveLastError();
 
 memset(, 0, sizeof(rerr));
 stream->closed = true;
 virStreamEventRemoveCallback(stream->st);
 virStreamAbort(stream->st);
-if (events & VIR_STREAM_EVENT_HANGUP)
-virReportError(VIR_ERR_RPC,
-   "%s", _("stream had unexpected termination"));
-else
-virReportError(VIR_ERR_RPC,
-   "%s", _("stream had I/O failure"));
+if (origErr && origErr->code != VIR_ERR_OK) {
+virSetError(origErr);
+virFreeError(origErr);
+} else {
+if (events & VIR_STREAM_EVENT_HANGUP)
+virReportError(VIR_ERR_RPC,
+   "%s", _("stream had unexpected termination"));
+else
+virReportError(VIR_ERR_RPC,
+   "%s", _("stream had I/O failure"));
+}
 
 msg = virNetMessageNew(false);
 if (!msg) {
diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index ebd0f6cf1..c1dbe0800 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -106,7 +106,7 @@ struct virFDStreamData {
 /* Thread data */
 virThreadPtr thread;
 virCond threadCond;
-int threadErr;
+virErrorPtr threadErr;
 bool threadQuit;
 bool threadAbort;
 bool threadDoRead;
@@ -123,6 +123,7 @@ virFDStreamDataDispose(void *obj)
 virFDStreamDataPtr fdst = obj;
 
 VIR_DEBUG("obj=%p", fdst);
+virFreeError(fdst->threadErr);
 virFDStreamMsgQueueFree(>msg);
 }
 
@@ -312,8 +313,10 @@ static void virFDStreamEvent(int watch ATTRIBUTE_UNUSED,
 return;
 }
 
-if (fdst->threadErr)
+if (fdst->threadErr) {
 events |= VIR_STREAM_EVENT_ERROR;
+virSetError(fdst->threadErr);
+}
 
 cb = fdst->cb;
 cbopaque = fdst->opaque;
@@ -637,7 +640,7 @@ virFDStreamThread(void *opaque)
 return;
 
  error:
-fdst->threadErr = errno;
+fdst->threadErr = virSaveLastError();
 goto cleanup;
 }
 
-- 
2.13.0

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

[libvirt] [PATCH 0/2] Two simple sparse streams fixes

[Michal Privoznik]

I've been experimenting with sparse streams and found a bug. If you try to
download a volume which doesn't support sparseness here's what happens:

# virsh vol-download --sparse 
/dev/disk/by-path/ip-XX.XX.XX.XX:3260-iscsi-iqn.2017-03.com.blah:server-lun-0 
/mnt/floppy/blah.raw

# echo $?
0
# ls -lhs /mnt/floppy/bla.raw
0 -rw-r--r-- 1 root root 0 May 30 12:40 /mnt/floppy/bla.raw

That's not good. iSCSI doesn't know anything about sparseness so an error is
expected here. Fortunately, the fix is fairly simple:

# virsh vol-download --sparse 
/dev/disk/by-path/ip-XX.XX.XX.XX:3260-iscsi-iqn.2017-03.com.blah:server-lun-0 
/mnt/floppy/bla.raw
error: cannot close volume 
/dev/disk/by-path/ip-XX.XX.XX.XX:3260-iscsi-iqn.2017-03.com.blah:server-lun-0
error: Unable to seek to data: Invalid argument


Michal Privoznik (2):
  virfdstream: Check for thread error more frequently
  fdstream: Report error from the I/O thread

 daemon/stream.c| 18 --
 src/util/virfdstream.c | 22 --
 2 files changed, 32 insertions(+), 8 deletions(-)

-- 
2.13.0

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

[libvirt] [PATCH v3 9/9] interface: Convert virInterfaceObj to use virObjectLockable

[John Ferlan]

Now that we have a bit more control, let's convert our object into
a lockable object and let that magic handle the create and lock/unlock.

This commit also introduces virInterfaceObjEndAPI in order to handle the
lock unlock and object unref in one call for consumers returning a NULL
obj upon return. This removes the need for virInterfaceObj{Lock|Unlock}
external API's.

Signed-off-by: John Ferlan 
---
 po/POTFILES.in |   1 -
 src/conf/virinterfaceobj.c | 105 -
 src/conf/virinterfaceobj.h |   9 ++--
 src/libvirt_private.syms   |   3 +-
 src/test/test_driver.c |  15 +++
 5 files changed, 68 insertions(+), 65 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 142b4d3..923d647 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -44,7 +44,6 @@ src/conf/storage_adapter_conf.c
 src/conf/storage_conf.c
 src/conf/virchrdev.c
 src/conf/virdomainobjlist.c
-src/conf/virinterfaceobj.c
 src/conf/virnetworkobj.c
 src/conf/virnodedeviceobj.c
 src/conf/virnwfilterobj.c
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 1e3f25c..51c3c82 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -33,7 +33,7 @@
 VIR_LOG_INIT("conf.virinterfaceobj");
 
 struct _virInterfaceObj {
-virMutex lock;
+virObjectLockable parent;
 
 bool active;   /* true if interface is active (up) */
 virInterfaceDefPtr def; /* The interface definition */
@@ -46,50 +46,60 @@ struct _virInterfaceObjList {
 
 /* virInterfaceObj manipulation */
 
-static virInterfaceObjPtr
-virInterfaceObjNew(void)
-{
-virInterfaceObjPtr obj;
+static virClassPtr virInterfaceObjClass;
+static void virInterfaceObjDispose(void *obj);
 
-if (VIR_ALLOC(obj) < 0)
-return NULL;
+static int
+virInterfaceObjOnceInit(void)
+{
+if (!(virInterfaceObjClass = virClassNew(virClassForObjectLockable(),
+ "virInterfaceObj",
+ sizeof(virInterfaceObj),
+ virInterfaceObjDispose)))
+return -1;
 
-if (virMutexInit(>lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("cannot initialize mutex"));
-VIR_FREE(obj);
-return NULL;
-}
+return 0;
+}
 
-virInterfaceObjLock(obj);
 
-return obj;
-}
+VIR_ONCE_GLOBAL_INIT(virInterfaceObj)
 
 
-void
-virInterfaceObjLock(virInterfaceObjPtr obj)
+static void
+virInterfaceObjDispose(void *opaque)
 {
-virMutexLock(>lock);
+virInterfaceObjPtr obj = opaque;
+
+virInterfaceDefFree(obj->def);
 }
 
 
-void
-virInterfaceObjUnlock(virInterfaceObjPtr obj)
+static virInterfaceObjPtr
+virInterfaceObjNew(void)
 {
-virMutexUnlock(>lock);
+virInterfaceObjPtr obj;
+
+if (virInterfaceObjInitialize() < 0)
+return NULL;
+
+if (!(obj = virObjectLockableNew(virInterfaceObjClass)))
+return NULL;
+
+virObjectLock(obj);
+
+return obj;
 }
 
 
 void
-virInterfaceObjFree(virInterfaceObjPtr obj)
+virInterfaceObjEndAPI(virInterfaceObjPtr *obj)
 {
-if (!obj)
+if (!*obj)
 return;
 
-virInterfaceDefFree(obj->def);
-virMutexDestroy(>lock);
-VIR_FREE(obj);
+virObjectUnlock(*obj);
+virObjectUnref(*obj);
+*obj = NULL;
 }
 
 
@@ -140,18 +150,18 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr 
interfaces,
 virInterfaceObjPtr obj = interfaces->objs[i];
 virInterfaceDefPtr def;
 
-virInterfaceObjLock(obj);
+virObjectLock(obj);
 def = obj->def;
 if (STRCASEEQ(def->mac, mac)) {
 if (matchct < maxmatches) {
 if (VIR_STRDUP(matches[matchct], def->name) < 0) {
-virInterfaceObjUnlock(obj);
+virObjectUnlock(obj);
 goto error;
 }
 matchct++;
 }
 }
-virInterfaceObjUnlock(obj);
+virObjectUnlock(obj);
 }
 return matchct;
 
@@ -173,11 +183,11 @@ virInterfaceObjListFindByName(virInterfaceObjListPtr 
interfaces,
 virInterfaceObjPtr obj = interfaces->objs[i];
 virInterfaceDefPtr def;
 
-virInterfaceObjLock(obj);
+virObjectLock(obj);
 def = obj->def;
 if (STREQ(def->name, name))
-return obj;
-virInterfaceObjUnlock(obj);
+return virObjectRef(obj);
+virObjectUnlock(obj);
 }
 
 return NULL;
@@ -190,7 +200,7 @@ virInterfaceObjListFree(virInterfaceObjListPtr interfaces)
 size_t i;
 
 for (i = 0; i < interfaces->count; i++)
-virInterfaceObjFree(interfaces->objs[i]);
+virObjectUnref(interfaces->objs[i]);
 VIR_FREE(interfaces->objs);
 VIR_FREE(interfaces);
 }
@@ -227,7 +237,7 @@ virInterfaceObjListClone(virInterfaceObjListPtr interfaces)
 VIR_FREE(xml);
 if (!(obj = 

[libvirt] [PATCH 1/2] virfdstream: Check for thread error more frequently

[Michal Privoznik]

When the I/O thread quits (e.g. due to an I/O error, lseek()
error, whatever), any subsequent virFDStream API should return
error too. Moreover, when invoking stream event callback, we must
set the VIR_STREAM_EVENT_ERROR flag so that the callback knows
something bad happened.

Signed-off-by: Michal Privoznik 
---
 src/util/virfdstream.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index 7ee58be13..ebd0f6cf1 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -312,6 +312,9 @@ static void virFDStreamEvent(int watch ATTRIBUTE_UNUSED,
 return;
 }
 
+if (fdst->threadErr)
+events |= VIR_STREAM_EVENT_ERROR;
+
 cb = fdst->cb;
 cbopaque = fdst->opaque;
 ff = fdst->ff;
@@ -764,6 +767,9 @@ static int virFDStreamWrite(virStreamPtr st, const char 
*bytes, size_t nbytes)
 return -1;
 }
 
+if (fdst->threadErr)
+return -1;
+
 if (!fdst) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("stream is not open"));
@@ -844,6 +850,9 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, 
size_t nbytes)
 return -1;
 }
 
+if (fdst->threadErr)
+return -1;
+
 if (!fdst) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("stream is not open"));
@@ -959,6 +968,9 @@ virFDStreamSendHole(virStreamPtr st,
 fdst->offset += length;
 }
 
+if (fdst->threadErr)
+goto cleanup;
+
 if (fdst->thread) {
 /* Things are a bit complicated here. If FDStream is in a
  * read mode, then if the message at the queue head is
@@ -1018,6 +1030,9 @@ virFDStreamInData(virStreamPtr st,
 
 virObjectLock(fdst);
 
+if (fdst->threadErr)
+goto cleanup;
+
 if (fdst->thread) {
 virFDStreamMsgPtr msg;
 
-- 
2.13.0

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

[libvirt] [PATCH v3 6/9] interface: Rename some virInterfaceObj* API's

[John Ferlan]

Prefix should have been virInterfaceObjList since the API is operating
on the list of interfaces.

Signed-off-by: John Ferlan 
---
 src/conf/virinterfaceobj.c | 37 +++--
 src/conf/virinterfaceobj.h | 31 ---
 src/libvirt_private.syms   | 12 ++--
 src/test/test_driver.c | 20 +++-
 4 files changed, 52 insertions(+), 48 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index dd86151..9470b4a 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -107,9 +107,10 @@ virInterfaceObjListNew(void)
 
 
 int
-virInterfaceObjFindByMACString(virInterfaceObjListPtr interfaces,
-   const char *mac,
-   virInterfaceObjPtr *matches, int maxmatches)
+virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
+   const char *mac,
+   virInterfaceObjPtr *matches,
+   int maxmatches)
 {
 size_t i;
 unsigned int matchct = 0;
@@ -137,8 +138,8 @@ virInterfaceObjFindByMACString(virInterfaceObjListPtr 
interfaces,
 
 
 virInterfaceObjPtr
-virInterfaceObjFindByName(virInterfaceObjListPtr interfaces,
-  const char *name)
+virInterfaceObjListFindByName(virInterfaceObjListPtr interfaces,
+  const char *name)
 {
 size_t i;
 
@@ -198,9 +199,9 @@ virInterfaceObjListClone(virInterfaceObjListPtr interfaces)
 }
 
 VIR_FREE(xml);
-if (!(obj = virInterfaceObjAssignDef(dest, backup)))
+if (!(obj = virInterfaceObjListAssignDef(dest, backup)))
 goto error;
-virInterfaceObjUnlock(obj); /* locked by virInterfaceObjAssignDef */
+virInterfaceObjUnlock(obj); /* locked by virInterfaceObjListAssignDef 
*/
 }
 
 return dest;
@@ -212,12 +213,12 @@ virInterfaceObjListClone(virInterfaceObjListPtr 
interfaces)
 
 
 virInterfaceObjPtr
-virInterfaceObjAssignDef(virInterfaceObjListPtr interfaces,
- virInterfaceDefPtr def)
+virInterfaceObjListAssignDef(virInterfaceObjListPtr interfaces,
+ virInterfaceDefPtr def)
 {
 virInterfaceObjPtr obj;
 
-if ((obj = virInterfaceObjFindByName(interfaces, def->name))) {
+if ((obj = virInterfaceObjListFindByName(interfaces, def->name))) {
 virInterfaceDefFree(obj->def);
 obj->def = def;
 
@@ -247,8 +248,8 @@ virInterfaceObjAssignDef(virInterfaceObjListPtr interfaces,
 
 
 void
-virInterfaceObjRemove(virInterfaceObjListPtr interfaces,
-  virInterfaceObjPtr obj)
+virInterfaceObjListRemove(virInterfaceObjListPtr interfaces,
+  virInterfaceObjPtr obj)
 {
 size_t i;
 
@@ -268,8 +269,8 @@ virInterfaceObjRemove(virInterfaceObjListPtr interfaces,
 
 
 int
-virInterfaceObjNumOfInterfaces(virInterfaceObjListPtr interfaces,
-   bool wantActive)
+virInterfaceObjListNumOfInterfaces(virInterfaceObjListPtr interfaces,
+   bool wantActive)
 {
 size_t i;
 int ninterfaces = 0;
@@ -287,10 +288,10 @@ virInterfaceObjNumOfInterfaces(virInterfaceObjListPtr 
interfaces,
 
 
 int
-virInterfaceObjGetNames(virInterfaceObjListPtr interfaces,
-bool wantActive,
-char **const names,
-int maxnames)
+virInterfaceObjListGetNames(virInterfaceObjListPtr interfaces,
+bool wantActive,
+char **const names,
+int maxnames)
 {
 int nnames = 0;
 size_t i;
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index 19c4947..f1bcab9 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -42,13 +42,14 @@ virInterfaceObjListPtr
 virInterfaceObjListNew(void);
 
 int
-virInterfaceObjFindByMACString(virInterfaceObjListPtr interfaces,
-   const char *mac,
-   virInterfaceObjPtr *matches, int maxmatches);
+virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
+   const char *mac,
+   virInterfaceObjPtr *matches,
+   int maxmatches);
 
 virInterfaceObjPtr
-virInterfaceObjFindByName(virInterfaceObjListPtr interfaces,
-  const char *name);
+virInterfaceObjListFindByName(virInterfaceObjListPtr interfaces,
+  const char *name);
 
 void
 virInterfaceObjFree(virInterfaceObjPtr obj);
@@ -60,12 +61,12 @@ virInterfaceObjListPtr
 virInterfaceObjListClone(virInterfaceObjListPtr interfaces);
 
 virInterfaceObjPtr
-virInterfaceObjAssignDef(virInterfaceObjListPtr interfaces,
- virInterfaceDefPtr def);

[libvirt] [PATCH v3 5/9] interface: Make _virInterfaceObjList struct private

[John Ferlan]

Move the structs into virinterfaceobj.c, create necessary accessors, and
initializers.

This also includes reworking virInterfaceObjListClone to handle receiving
a source interfaces list pointer, creating the destination interfaces object,
and copying everything from source into dest.

Signed-off-by: John Ferlan 
---
 src/conf/virinterfaceobj.c | 57 +-
 src/conf/virinterfaceobj.h | 12 --
 src/libvirt_private.syms   |  1 +
 src/test/test_driver.c | 38 +++
 4 files changed, 60 insertions(+), 48 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index a2ef7f4..dd86151 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -39,6 +39,10 @@ struct _virInterfaceObj {
 virInterfaceDefPtr def; /* The interface definition */
 };
 
+struct _virInterfaceObjList {
+size_t count;
+virInterfaceObjPtr *objs;
+};
 
 /* virInterfaceObj manipulation */
 
@@ -91,6 +95,17 @@ virInterfaceObjSetActive(virInterfaceObjPtr obj,
 
 
 /* virInterfaceObjList manipulation */
+virInterfaceObjListPtr
+virInterfaceObjListNew(void)
+{
+virInterfaceObjListPtr interfaces;
+
+if (VIR_ALLOC(interfaces) < 0)
+return NULL;
+return interfaces;
+}
+
+
 int
 virInterfaceObjFindByMACString(virInterfaceObjListPtr interfaces,
const char *mac,
@@ -149,50 +164,50 @@ virInterfaceObjListFree(virInterfaceObjListPtr interfaces)
 
 for (i = 0; i < interfaces->count; i++)
 virInterfaceObjFree(interfaces->objs[i]);
-
 VIR_FREE(interfaces->objs);
-interfaces->count = 0;
+VIR_FREE(interfaces);
 }
 
 
-int
-virInterfaceObjListClone(virInterfaceObjListPtr src,
- virInterfaceObjListPtr dest)
+virInterfaceObjListPtr
+virInterfaceObjListClone(virInterfaceObjListPtr interfaces)
 {
-int ret = -1;
 size_t i;
 unsigned int cnt;
+virInterfaceObjListPtr dest;
 
-if (!src || !dest)
-goto cleanup;
+if (!interfaces)
+return NULL;
 
-virInterfaceObjListFree(dest); /* start with an empty list */
-cnt = src->count;
+if (!(dest = virInterfaceObjListNew()))
+return NULL;
+
+cnt = interfaces->count;
 for (i = 0; i < cnt; i++) {
-virInterfaceObjPtr srcobj = src->objs[i];
+virInterfaceObjPtr srcobj = interfaces->objs[i];
 virInterfaceDefPtr backup;
 virInterfaceObjPtr obj;
 char *xml = virInterfaceDefFormat(srcobj->def);
 
 if (!xml)
-goto cleanup;
+goto error;
 
-if ((backup = virInterfaceDefParseString(xml)) == NULL) {
+if (!(backup = virInterfaceDefParseString(xml))) {
 VIR_FREE(xml);
-goto cleanup;
+goto error;
 }
 
 VIR_FREE(xml);
-if ((obj = virInterfaceObjAssignDef(dest, backup)) == NULL)
-goto cleanup;
+if (!(obj = virInterfaceObjAssignDef(dest, backup)))
+goto error;
 virInterfaceObjUnlock(obj); /* locked by virInterfaceObjAssignDef */
 }
 
-ret = cnt;
- cleanup:
-if ((ret < 0) && dest)
-   virInterfaceObjListFree(dest);
-return ret;
+return dest;
+
+ error:
+virInterfaceObjListFree(dest);
+return NULL;
 }
 
 
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index 79b6fc9..19c4947 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -27,10 +27,6 @@ typedef virInterfaceObj *virInterfaceObjPtr;
 
 typedef struct _virInterfaceObjList virInterfaceObjList;
 typedef virInterfaceObjList *virInterfaceObjListPtr;
-struct _virInterfaceObjList {
-size_t count;
-virInterfaceObjPtr *objs;
-};
 
 virInterfaceDefPtr
 virInterfaceObjGetDef(virInterfaceObjPtr obj);
@@ -42,6 +38,9 @@ void
 virInterfaceObjSetActive(virInterfaceObjPtr obj,
  bool active);
 
+virInterfaceObjListPtr
+virInterfaceObjListNew(void);
+
 int
 virInterfaceObjFindByMACString(virInterfaceObjListPtr interfaces,
const char *mac,
@@ -57,9 +56,8 @@ virInterfaceObjFree(virInterfaceObjPtr obj);
 void
 virInterfaceObjListFree(virInterfaceObjListPtr vms);
 
-int
-virInterfaceObjListClone(virInterfaceObjListPtr src,
- virInterfaceObjListPtr dest);
+virInterfaceObjListPtr
+virInterfaceObjListClone(virInterfaceObjListPtr interfaces);
 
 virInterfaceObjPtr
 virInterfaceObjAssignDef(virInterfaceObjListPtr interfaces,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0929728..bdf4eeb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -918,6 +918,7 @@ virInterfaceObjGetNames;
 virInterfaceObjIsActive;
 virInterfaceObjListClone;
 virInterfaceObjListFree;
+virInterfaceObjListNew;
 virInterfaceObjLock;
 virInterfaceObjNumOfInterfaces;
 virInterfaceObjRemove;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c

[libvirt] [PATCH v3 2/9] interface: Remove some unnecessary goto's for Interface tests

[John Ferlan]

Rather than using goto cleanup on object find failure and having cleanup
need to check if the obj was present before unlocking, just return immediately.

Signed-off-by: John Ferlan 
---
 src/test/test_driver.c | 35 ---
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d9f9329..8f7ff63 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3709,13 +3709,11 @@ testInterfaceLookupByName(virConnectPtr conn,
 virInterfacePtr ret = NULL;
 
 if (!(obj = testInterfaceObjFindByName(privconn, name)))
-goto cleanup;
+return NULL;
 
 ret = virGetInterface(conn, obj->def->name, obj->def->mac);
 
- cleanup:
-if (obj)
-virInterfaceObjUnlock(obj);
+virInterfaceObjUnlock(obj);
 return ret;
 }
 
@@ -3760,13 +3758,11 @@ testInterfaceIsActive(virInterfacePtr iface)
 int ret = -1;
 
 if (!(obj = testInterfaceObjFindByName(privconn, iface->name)))
-goto cleanup;
+return -1;
 
 ret = virInterfaceObjIsActive(obj);
 
- cleanup:
-if (obj)
-virInterfaceObjUnlock(obj);
+virInterfaceObjUnlock(obj);
 return ret;
 }
 
@@ -3875,13 +3871,11 @@ testInterfaceGetXMLDesc(virInterfacePtr iface,
 virCheckFlags(0, NULL);
 
 if (!(obj = testInterfaceObjFindByName(privconn, iface->name)))
-goto cleanup;
+return NULL;
 
 ret = virInterfaceDefFormat(obj->def);
 
- cleanup:
-if (obj)
-virInterfaceObjUnlock(obj);
+virInterfaceObjUnlock(obj);
 return ret;
 }
 
@@ -3922,16 +3916,13 @@ testInterfaceUndefine(virInterfacePtr iface)
 {
 testDriverPtr privconn = iface->conn->privateData;
 virInterfaceObjPtr obj;
-int ret = -1;
 
 if (!(obj = testInterfaceObjFindByName(privconn, iface->name)))
-goto cleanup;
+return -1;
 
 virInterfaceObjRemove(>ifaces, obj);
-ret = 0;
 
- cleanup:
-return ret;
+return 0;
 }
 
 
@@ -3946,7 +3937,7 @@ testInterfaceCreate(virInterfacePtr iface,
 virCheckFlags(0, -1);
 
 if (!(obj = testInterfaceObjFindByName(privconn, iface->name)))
-goto cleanup;
+return -1;
 
 if (obj->active != 0) {
 virReportError(VIR_ERR_OPERATION_INVALID, NULL);
@@ -3957,8 +3948,7 @@ testInterfaceCreate(virInterfacePtr iface,
 ret = 0;
 
  cleanup:
-if (obj)
-virInterfaceObjUnlock(obj);
+virInterfaceObjUnlock(obj);
 return ret;
 }
 
@@ -3974,7 +3964,7 @@ testInterfaceDestroy(virInterfacePtr iface,
 virCheckFlags(0, -1);
 
 if (!(obj = testInterfaceObjFindByName(privconn, iface->name)))
-goto cleanup;
+return -1;
 
 if (obj->active == 0) {
 virReportError(VIR_ERR_OPERATION_INVALID, NULL);
@@ -3985,8 +3975,7 @@ testInterfaceDestroy(virInterfacePtr iface,
 ret = 0;
 
  cleanup:
-if (obj)
-virInterfaceObjUnlock(obj);
+virInterfaceObjUnlock(obj);
 return ret;
 }
 
-- 
2.9.4

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

[libvirt] [PATCH v3 3/9] interface: Use virInterfaceDefPtr rather than deref from virInterfaceObjPtr

[John Ferlan]

We're about to make the obj much more private, so make it easier to
see future changes which will require accessors for the obj->def

This also includes modifying some interfaces->objs[i]->X references to be
obj = interfaces->objs[i]; and then def = obj->def

Signed-off-by: John Ferlan 
---
 src/conf/virinterfaceobj.c | 32 +---
 src/test/test_driver.c | 12 +---
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 62c3735..ead9512 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -72,18 +72,21 @@ virInterfaceObjFindByMACString(virInterfaceObjListPtr 
interfaces,
 unsigned int matchct = 0;
 
 for (i = 0; i < interfaces->count; i++) {
+virInterfaceObjPtr obj = interfaces->objs[i];
+virInterfaceDefPtr def;
 
-virInterfaceObjLock(interfaces->objs[i]);
-if (STRCASEEQ(interfaces->objs[i]->def->mac, mac)) {
+virInterfaceObjLock(obj);
+def = obj->def;
+if (STRCASEEQ(def->mac, mac)) {
 matchct++;
 if (matchct <= maxmatches) {
-matches[matchct - 1] = interfaces->objs[i];
+matches[matchct - 1] = obj;
 /* keep the lock if we're returning object to caller */
 /* it is the caller's responsibility to unlock *all* matches */
 continue;
 }
 }
-virInterfaceObjUnlock(interfaces->objs[i]);
+virInterfaceObjUnlock(obj);
 
 }
 return matchct;
@@ -97,10 +100,14 @@ virInterfaceObjFindByName(virInterfaceObjListPtr 
interfaces,
 size_t i;
 
 for (i = 0; i < interfaces->count; i++) {
-virInterfaceObjLock(interfaces->objs[i]);
-if (STREQ(interfaces->objs[i]->def->name, name))
-return interfaces->objs[i];
-virInterfaceObjUnlock(interfaces->objs[i]);
+virInterfaceObjPtr obj = interfaces->objs[i];
+virInterfaceDefPtr def;
+
+virInterfaceObjLock(obj);
+def = obj->def;
+if (STREQ(def->name, name))
+return obj;
+virInterfaceObjUnlock(obj);
 }
 
 return NULL;
@@ -134,10 +141,10 @@ virInterfaceObjListClone(virInterfaceObjListPtr src,
 virInterfaceObjListFree(dest); /* start with an empty list */
 cnt = src->count;
 for (i = 0; i < cnt; i++) {
-virInterfaceDefPtr def = src->objs[i]->def;
+virInterfaceObjPtr srcobj = src->objs[i];
 virInterfaceDefPtr backup;
 virInterfaceObjPtr obj;
-char *xml = virInterfaceDefFormat(def);
+char *xml = virInterfaceDefFormat(srcobj->def);
 
 if (!xml)
 goto cleanup;
@@ -247,9 +254,12 @@ virInterfaceObjGetNames(virInterfaceObjListPtr interfaces,
 
 for (i = 0; i < interfaces->count && nnames < maxnames; i++) {
 virInterfaceObjPtr obj = interfaces->objs[i];
+virInterfaceDefPtr def;
+
 virInterfaceObjLock(obj);
+def = obj->def;
 if (wantActive == virInterfaceObjIsActive(obj)) {
-if (VIR_STRDUP(names[nnames], obj->def->name) < 0) {
+if (VIR_STRDUP(names[nnames], def->name) < 0) {
 virInterfaceObjUnlock(obj);
 goto failure;
 }
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 8f7ff63..29c31ad 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3706,12 +3706,14 @@ testInterfaceLookupByName(virConnectPtr conn,
 {
 testDriverPtr privconn = conn->privateData;
 virInterfaceObjPtr obj;
+virInterfaceDefPtr def;
 virInterfacePtr ret = NULL;
 
 if (!(obj = testInterfaceObjFindByName(privconn, name)))
 return NULL;
+def = obj->def;
 
-ret = virGetInterface(conn, obj->def->name, obj->def->mac);
+ret = virGetInterface(conn, def->name, def->mac);
 
 virInterfaceObjUnlock(obj);
 return ret;
@@ -3724,6 +3726,7 @@ testInterfaceLookupByMACString(virConnectPtr conn,
 {
 testDriverPtr privconn = conn->privateData;
 virInterfaceObjPtr obj;
+virInterfaceDefPtr def;
 int ifacect;
 virInterfacePtr ret = NULL;
 
@@ -3741,7 +3744,8 @@ testInterfaceLookupByMACString(virConnectPtr conn,
 goto cleanup;
 }
 
-ret = virGetInterface(conn, obj->def->name, obj->def->mac);
+def = obj->def;
+ret = virGetInterface(conn, def->name, def->mac);
 
  cleanup:
 if (obj)
@@ -3888,6 +3892,7 @@ testInterfaceDefineXML(virConnectPtr conn,
 testDriverPtr privconn = conn->privateData;
 virInterfaceDefPtr def;
 virInterfaceObjPtr obj = NULL;
+virInterfaceDefPtr objdef;
 virInterfacePtr ret = NULL;
 
 virCheckFlags(0, NULL);
@@ -3899,8 +3904,9 @@ testInterfaceDefineXML(virConnectPtr conn,
 if ((obj = virInterfaceObjAssignDef(>ifaces, def)) == NULL)
 goto cleanup;
 def = NULL;
+objdef = obj->def;
 
-ret = virGetInterface(conn, 

[libvirt] [PATCH v3 1/9] interface: Consistently use 'obj' for a virInterfaceObjPtr

[John Ferlan]

Alter variable names to be obj rather than 'iface' and/or 'obj'.

Signed-off-by: John Ferlan 
---
 src/conf/virinterfaceobj.c | 48 ++--
 src/conf/virinterfaceobj.h |  4 +--
 src/test/test_driver.c | 78 +++---
 3 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index e80db23..62c3735 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -51,14 +51,14 @@ virInterfaceObjUnlock(virInterfaceObjPtr obj)
 
 
 void
-virInterfaceObjFree(virInterfaceObjPtr iface)
+virInterfaceObjFree(virInterfaceObjPtr obj)
 {
-if (!iface)
+if (!obj)
 return;
 
-virInterfaceDefFree(iface->def);
-virMutexDestroy(>lock);
-VIR_FREE(iface);
+virInterfaceDefFree(obj->def);
+virMutexDestroy(>lock);
+VIR_FREE(obj);
 }
 
 
@@ -136,7 +136,7 @@ virInterfaceObjListClone(virInterfaceObjListPtr src,
 for (i = 0; i < cnt; i++) {
 virInterfaceDefPtr def = src->objs[i]->def;
 virInterfaceDefPtr backup;
-virInterfaceObjPtr iface;
+virInterfaceObjPtr obj;
 char *xml = virInterfaceDefFormat(def);
 
 if (!xml)
@@ -148,9 +148,9 @@ virInterfaceObjListClone(virInterfaceObjListPtr src,
 }
 
 VIR_FREE(xml);
-if ((iface = virInterfaceObjAssignDef(dest, backup)) == NULL)
+if ((obj = virInterfaceObjAssignDef(dest, backup)) == NULL)
 goto cleanup;
-virInterfaceObjUnlock(iface); /* locked by virInterfaceObjAssignDef */
+virInterfaceObjUnlock(obj); /* locked by virInterfaceObjAssignDef */
 }
 
 ret = cnt;
@@ -165,47 +165,47 @@ virInterfaceObjPtr
 virInterfaceObjAssignDef(virInterfaceObjListPtr interfaces,
  virInterfaceDefPtr def)
 {
-virInterfaceObjPtr iface;
+virInterfaceObjPtr obj;
 
-if ((iface = virInterfaceObjFindByName(interfaces, def->name))) {
-virInterfaceDefFree(iface->def);
-iface->def = def;
+if ((obj = virInterfaceObjFindByName(interfaces, def->name))) {
+virInterfaceDefFree(obj->def);
+obj->def = def;
 
-return iface;
+return obj;
 }
 
-if (VIR_ALLOC(iface) < 0)
+if (VIR_ALLOC(obj) < 0)
 return NULL;
-if (virMutexInit(>lock) < 0) {
+if (virMutexInit(>lock) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot initialize mutex"));
-VIR_FREE(iface);
+VIR_FREE(obj);
 return NULL;
 }
-virInterfaceObjLock(iface);
+virInterfaceObjLock(obj);
 
 if (VIR_APPEND_ELEMENT_COPY(interfaces->objs,
-interfaces->count, iface) < 0) {
-virInterfaceObjFree(iface);
+interfaces->count, obj) < 0) {
+virInterfaceObjFree(obj);
 return NULL;
 }
 
-iface->def = def;
-return iface;
+obj->def = def;
+return obj;
 
 }
 
 
 void
 virInterfaceObjRemove(virInterfaceObjListPtr interfaces,
-  virInterfaceObjPtr iface)
+  virInterfaceObjPtr obj)
 {
 size_t i;
 
-virInterfaceObjUnlock(iface);
+virInterfaceObjUnlock(obj);
 for (i = 0; i < interfaces->count; i++) {
 virInterfaceObjLock(interfaces->objs[i]);
-if (interfaces->objs[i] == iface) {
+if (interfaces->objs[i] == obj) {
 virInterfaceObjUnlock(interfaces->objs[i]);
 virInterfaceObjFree(interfaces->objs[i]);
 
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index 5b0527d..ee166c6 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -54,7 +54,7 @@ virInterfaceObjFindByName(virInterfaceObjListPtr interfaces,
   const char *name);
 
 void
-virInterfaceObjFree(virInterfaceObjPtr iface);
+virInterfaceObjFree(virInterfaceObjPtr obj);
 
 void
 virInterfaceObjListFree(virInterfaceObjListPtr vms);
@@ -69,7 +69,7 @@ virInterfaceObjAssignDef(virInterfaceObjListPtr interfaces,
 
 void
 virInterfaceObjRemove(virInterfaceObjListPtr interfaces,
-  virInterfaceObjPtr iface);
+  virInterfaceObjPtr obj);
 
 void
 virInterfaceObjLock(virInterfaceObjPtr obj);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 9330d9e..d9f9329 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3627,18 +3627,18 @@ static virInterfaceObjPtr
 testInterfaceObjFindByName(testDriverPtr privconn,
const char *name)
 {
-virInterfaceObjPtr iface;
+virInterfaceObjPtr obj;
 
 testDriverLock(privconn);
-iface = virInterfaceObjFindByName(>ifaces, name);
+obj = virInterfaceObjFindByName(>ifaces, name);
 testDriverUnlock(privconn);
 
-if (!iface)
+if (!obj)
 virReportError(VIR_ERR_NO_INTERFACE,
_("no interface with 

[libvirt] [PATCH v3 0/9] Multiple cleanups within interfaceobj and interface driver

[John Ferlan]

v2: https://www.redhat.com/archives/libvir-list/2017-May/msg01059.html
v1: https://www.redhat.com/archives/libvir-list/2017-April/msg01225.html

Patches 1-7 were acked in v1 with a request to "show" the next step in
a 9/8 patch to convert to using virObject for virInterfaceObj.  v2 added
that, but also added a couple of patches that were unfavorable.

Since I didn't want to push something partial and it was later in the
release cycle, I didn't push 1-7. This series keeps the v1 patch 8 in
tact and adds patch9 which does the virObject conversion and the
virInterfaceObjEndAPI in the same patch (v2 separated those, but it seeems
Peter wasn't favoring that option, so I combined them).

I know this is not 3.4 material, but with a successful review of 8 & 9,
I would push this after 3.4 is cut.

John Ferlan (9):
  interface: Consistently use 'obj' for a virInterfaceObjPtr
  interface: Remove some unnecessary goto's for Interface tests
  interface: Use virInterfaceDefPtr rather than deref from
virInterfaceObjPtr
  interface: Make _virInterfaceObj struct private
  interface: Make _virInterfaceObjList struct private
  interface: Rename some virInterfaceObj* API's
  interface: Clean up virInterfaceObjListFindByMACString
  interface: Introduce virInterfaceObjNew
  interface: Convert virInterfaceObj to use virObjectLockable

 po/POTFILES.in |   1 -
 src/conf/virinterfaceobj.c | 275 +
 src/conf/virinterfaceobj.h |  72 ++--
 src/libvirt_private.syms   |  19 ++--
 src/test/test_driver.c | 142 ---
 5 files changed, 292 insertions(+), 217 deletions(-)

-- 
2.9.4

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

[libvirt] [PATCH v3 8/9] interface: Introduce virInterfaceObjNew

[John Ferlan]

Create/use a helper to perform the object allocation

Signed-off-by: John Ferlan 
---
 src/conf/virinterfaceobj.c | 31 +++
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 8bd8094..1e3f25c 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -46,6 +46,27 @@ struct _virInterfaceObjList {
 
 /* virInterfaceObj manipulation */
 
+static virInterfaceObjPtr
+virInterfaceObjNew(void)
+{
+virInterfaceObjPtr obj;
+
+if (VIR_ALLOC(obj) < 0)
+return NULL;
+
+if (virMutexInit(>lock) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("cannot initialize mutex"));
+VIR_FREE(obj);
+return NULL;
+}
+
+virInterfaceObjLock(obj);
+
+return obj;
+}
+
+
 void
 virInterfaceObjLock(virInterfaceObjPtr obj)
 {
@@ -230,18 +251,12 @@ virInterfaceObjListAssignDef(virInterfaceObjListPtr 
interfaces,
 return obj;
 }
 
-if (VIR_ALLOC(obj) < 0)
-return NULL;
-if (virMutexInit(>lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("cannot initialize mutex"));
-VIR_FREE(obj);
+if (!(obj = virInterfaceObjNew()))
 return NULL;
-}
-virInterfaceObjLock(obj);
 
 if (VIR_APPEND_ELEMENT_COPY(interfaces->objs,
 interfaces->count, obj) < 0) {
+virInterfaceObjUnlock(obj);
 virInterfaceObjFree(obj);
 return NULL;
 }
-- 
2.9.4

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

[libvirt] [PATCH v3 4/9] interface: Make _virInterfaceObj struct private

[John Ferlan]

Move the struct into virinterfaceobj.c, create necessary accessors, and
initializers.

Signed-off-by: John Ferlan 
---
 src/conf/virinterfaceobj.c | 28 
 src/conf/virinterfaceobj.h | 20 +---
 src/libvirt_private.syms   |  3 +++
 src/test/test_driver.c | 20 +++-
 4 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index ead9512..a2ef7f4 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -32,6 +32,12 @@
 
 VIR_LOG_INIT("conf.virinterfaceobj");
 
+struct _virInterfaceObj {
+virMutex lock;
+
+bool active;   /* true if interface is active (up) */
+virInterfaceDefPtr def; /* The interface definition */
+};
 
 
 /* virInterfaceObj manipulation */
@@ -62,6 +68,28 @@ virInterfaceObjFree(virInterfaceObjPtr obj)
 }
 
 
+virInterfaceDefPtr
+virInterfaceObjGetDef(virInterfaceObjPtr obj)
+{
+return obj->def;
+}
+
+
+bool
+virInterfaceObjIsActive(virInterfaceObjPtr obj)
+{
+return obj->active;
+}
+
+
+void
+virInterfaceObjSetActive(virInterfaceObjPtr obj,
+ bool active)
+{
+obj->active = active;
+}
+
+
 /* virInterfaceObjList manipulation */
 int
 virInterfaceObjFindByMACString(virInterfaceObjListPtr interfaces,
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index ee166c6..79b6fc9 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -24,12 +24,6 @@
 
 typedef struct _virInterfaceObj virInterfaceObj;
 typedef virInterfaceObj *virInterfaceObjPtr;
-struct _virInterfaceObj {
-virMutex lock;
-
-bool active;   /* true if interface is active (up) */
-virInterfaceDefPtr def; /* The interface definition */
-};
 
 typedef struct _virInterfaceObjList virInterfaceObjList;
 typedef virInterfaceObjList *virInterfaceObjListPtr;
@@ -38,11 +32,15 @@ struct _virInterfaceObjList {
 virInterfaceObjPtr *objs;
 };
 
-static inline bool
-virInterfaceObjIsActive(const virInterfaceObj *iface)
-{
-return iface->active;
-}
+virInterfaceDefPtr
+virInterfaceObjGetDef(virInterfaceObjPtr obj);
+
+bool
+virInterfaceObjIsActive(virInterfaceObjPtr obj);
+
+void
+virInterfaceObjSetActive(virInterfaceObjPtr obj,
+ bool active);
 
 int
 virInterfaceObjFindByMACString(virInterfaceObjListPtr interfaces,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 429b095..0929728 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -913,12 +913,15 @@ virDomainObjListRename;
 virInterfaceObjAssignDef;
 virInterfaceObjFindByMACString;
 virInterfaceObjFindByName;
+virInterfaceObjGetDef;
 virInterfaceObjGetNames;
+virInterfaceObjIsActive;
 virInterfaceObjListClone;
 virInterfaceObjListFree;
 virInterfaceObjLock;
 virInterfaceObjNumOfInterfaces;
 virInterfaceObjRemove;
+virInterfaceObjSetActive;
 virInterfaceObjUnlock;
 
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 29c31ad..1b6063a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1025,7 +1025,7 @@ testParseInterfaces(testDriverPtr privconn,
 goto error;
 }
 
-obj->active = 1;
+virInterfaceObjSetActive(obj, true);
 virInterfaceObjUnlock(obj);
 }
 
@@ -3711,7 +3711,7 @@ testInterfaceLookupByName(virConnectPtr conn,
 
 if (!(obj = testInterfaceObjFindByName(privconn, name)))
 return NULL;
-def = obj->def;
+def = virInterfaceObjGetDef(obj);
 
 ret = virGetInterface(conn, def->name, def->mac);
 
@@ -3744,7 +3744,7 @@ testInterfaceLookupByMACString(virConnectPtr conn,
 goto cleanup;
 }
 
-def = obj->def;
+def = virInterfaceObjGetDef(obj);
 ret = virGetInterface(conn, def->name, def->mac);
 
  cleanup:
@@ -3870,14 +3870,16 @@ testInterfaceGetXMLDesc(virInterfacePtr iface,
 {
 testDriverPtr privconn = iface->conn->privateData;
 virInterfaceObjPtr obj;
+virInterfaceDefPtr def;
 char *ret = NULL;
 
 virCheckFlags(0, NULL);
 
 if (!(obj = testInterfaceObjFindByName(privconn, iface->name)))
 return NULL;
+def = virInterfaceObjGetDef(obj);
 
-ret = virInterfaceDefFormat(obj->def);
+ret = virInterfaceDefFormat(def);
 
 virInterfaceObjUnlock(obj);
 return ret;
@@ -3904,7 +3906,7 @@ testInterfaceDefineXML(virConnectPtr conn,
 if ((obj = virInterfaceObjAssignDef(>ifaces, def)) == NULL)
 goto cleanup;
 def = NULL;
-objdef = obj->def;
+objdef = virInterfaceObjGetDef(obj);
 
 ret = virGetInterface(conn, objdef->name, objdef->mac);
 
@@ -3945,12 +3947,12 @@ testInterfaceCreate(virInterfacePtr iface,
 if (!(obj = testInterfaceObjFindByName(privconn, iface->name)))
 return -1;
 
-if (obj->active != 0) {
+if (virInterfaceObjIsActive(obj)) {
 virReportError(VIR_ERR_OPERATION_INVALID, NULL);
 goto cleanup;
 }
 
-obj->active = 

[libvirt] [PATCH v3 7/9] interface: Clean up virInterfaceObjListFindByMACString

[John Ferlan]

Alter the algorithm to return a list of matching names rather than a
list of match virInterfaceObjPtr which are then just dereferenced
extracting the def->name and def->mac. Since the def->mac would be
the same as the passed @mac, just return a list of names and as long
as there's only one, extract the [0] entry from the passed list.
Also alter the error message on failure to include the mac that wasn't
found.

Signed-off-by: John Ferlan 
---
 src/conf/virinterfaceobj.c | 23 ++-
 src/conf/virinterfaceobj.h |  2 +-
 src/test/test_driver.c | 16 
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 9470b4a..8bd8094 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -109,11 +109,11 @@ virInterfaceObjListNew(void)
 int
 virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
const char *mac,
-   virInterfaceObjPtr *matches,
+   char **const matches,
int maxmatches)
 {
 size_t i;
-unsigned int matchct = 0;
+int matchct = 0;
 
 for (i = 0; i < interfaces->count; i++) {
 virInterfaceObjPtr obj = interfaces->objs[i];
@@ -122,18 +122,23 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr 
interfaces,
 virInterfaceObjLock(obj);
 def = obj->def;
 if (STRCASEEQ(def->mac, mac)) {
-matchct++;
-if (matchct <= maxmatches) {
-matches[matchct - 1] = obj;
-/* keep the lock if we're returning object to caller */
-/* it is the caller's responsibility to unlock *all* matches */
-continue;
+if (matchct < maxmatches) {
+if (VIR_STRDUP(matches[matchct], def->name) < 0) {
+virInterfaceObjUnlock(obj);
+goto error;
+}
+matchct++;
 }
 }
 virInterfaceObjUnlock(obj);
-
 }
 return matchct;
+
+ error:
+while (--matchct >= 0)
+VIR_FREE(matches[matchct]);
+
+return -1;
 }
 
 
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index f1bcab9..3934e63 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -44,7 +44,7 @@ virInterfaceObjListNew(void);
 int
 virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
const char *mac,
-   virInterfaceObjPtr *matches,
+   char **const matches,
int maxmatches);
 
 virInterfaceObjPtr
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 8444e91..511d65f 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3728,17 +3728,18 @@ testInterfaceLookupByMACString(virConnectPtr conn,
const char *mac)
 {
 testDriverPtr privconn = conn->privateData;
-virInterfaceObjPtr obj;
-virInterfaceDefPtr def;
 int ifacect;
+char *ifacenames[] = { NULL, NULL };
 virInterfacePtr ret = NULL;
 
 testDriverLock(privconn);
-ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, , 
1);
+ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac,
+ ifacenames, 2);
 testDriverUnlock(privconn);
 
 if (ifacect == 0) {
-virReportError(VIR_ERR_NO_INTERFACE, NULL);
+virReportError(VIR_ERR_NO_INTERFACE,
+   _("no interface with matching mac '%s'"), mac);
 goto cleanup;
 }
 
@@ -3747,12 +3748,11 @@ testInterfaceLookupByMACString(virConnectPtr conn,
 goto cleanup;
 }
 
-def = virInterfaceObjGetDef(obj);
-ret = virGetInterface(conn, def->name, def->mac);
+ret = virGetInterface(conn, ifacenames[0], mac);
 
  cleanup:
-if (obj)
-virInterfaceObjUnlock(obj);
+VIR_FREE(ifacenames[0]);
+VIR_FREE(ifacenames[1]);
 return ret;
 }
 
-- 
2.9.4

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

Re: [libvirt] [PATCH v2] nodedev: Increase the netlink socket buffer size to the one used by udev

[Daniel P. Berrange]

On Fri, May 19, 2017 at 03:16:26PM +0200, Erik Skultety wrote:
> From: "ning.bo" 
> 
> When a number of SRIOV VFs (up to 128 on Intel XL710) is created:
> for i in `seq 0 1`; do
>   echo 63 > /sys/class/net//device/sriov_numvfs
> done
> 
> libvirtd will then report "udev_monitor_receive_device returned NULL"
> error because the netlink socket buffer is not big enough (using GDB on
> libudev confirmed this with ENOBUFFS) and thus some udev events were
> dropped. This results in some devices being missing in the nodedev-list
> output. This patch overrides the system's rmem_max limit but for that,
> we need to make sure we've got root privileges.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1450960
> 
> Signed-off-by: ning.bo 
> Signed-off-by: Erik Skultety 
> ---
> Additionally, we might want to check for the errno, providing a specific
> message if such issue occurs again in a further non-specified point in time 
> and
> return the generic, yet cryptic one for all other cases.
> 
>  src/node_device/node_device_udev.c | 7 +++
>  1 file changed, 7 insertions(+)

This has broken the build on older systems  as udev doesn't have this
method.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] qemu:json: Fix daemon crash on handling domain shutdown event

[Erik Skultety]

On Tue, May 30, 2017 at 10:53:44AM +0200, Peter Krempa wrote:
> On Tue, May 30, 2017 at 10:41:17 +0200, Erik Skultety wrote:
> > commit a8eba5036 added further checking of the guest shutdown cause, but
> > this enhancement is available since qemu 2.10, causing a crash because
> > of a NULL pointer dereference on older qemus.
> >
> > Thread 1 "libvirtd" received signal SIGSEGV, Segmentation fault.
> > 0x772441af in virJSONValueObjectGet (object=0x0,
> >  key=0x7fffd5ef11bf "guest")
> > at util/virjson.c:769
> > 769 if (object->type != VIR_JSON_TYPE_OBJECT)
> > (gdb) bt
> > 0  in virJSONValueObjectGet
> > 1  in virJSONValueObjectGetBoolean
> > 2  in qemuMonitorJSONHandleShutdown
> > 3  in qemuMonitorJSONIOProcessEvent
> > 4  in qemuMonitorJSONIOProcessLine
> > 5  in qemuMonitorJSONIOProcess
> > 6  in qemuMonitorIOProcess
>
> I think you can truncate is somewhere here.
>
> > 7  in qemuMonitorIO
> > 8  in virEventPollDispatchHandles
> > 9  in virEventPollRunOnce
> > 10 in virEventRunDefaultImpl
> > 11 in virNetDaemonRun
> > 12 in main
> >
> > Signed-off-by: Erik Skultety 
> > ---
> >  src/qemu/qemu_monitor_json.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> > index 757595dd7..f208dd05a 100644
> > --- a/src/qemu/qemu_monitor_json.c
> > +++ b/src/qemu/qemu_monitor_json.c
> > @@ -528,7 +528,7 @@ static void 
> > qemuMonitorJSONHandleShutdown(qemuMonitorPtr mon, virJSONValuePtr da
> >  bool guest = false;
> >  virTristateBool guest_initiated = VIR_TRISTATE_BOOL_ABSENT;
> >
> > -if (virJSONValueObjectGetBoolean(data, "guest", ) == 0)
> > +if (data && virJSONValueObjectGetBoolean(data, "guest", ) == 0)
> >  guest_initiated = guest ? VIR_TRISTATE_BOOL_YES : 
> > VIR_TRISTATE_BOOL_NO;
> >
> >  qemuMonitorEmitShutdown(mon, guest_initiated);
>
> ACK, safe for freeze
>

Adjusted the commit message and pushed, thanks.
Erik



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

Re: [libvirt] [PATCH] qemu:json: Fix daemon crash on handling domain shutdown event

[Peter Krempa]

On Tue, May 30, 2017 at 10:41:17 +0200, Erik Skultety wrote:
> commit a8eba5036 added further checking of the guest shutdown cause, but
> this enhancement is available since qemu 2.10, causing a crash because
> of a NULL pointer dereference on older qemus.
> 
> Thread 1 "libvirtd" received signal SIGSEGV, Segmentation fault.
> 0x772441af in virJSONValueObjectGet (object=0x0,
>  key=0x7fffd5ef11bf "guest")
> at util/virjson.c:769
> 769   if (object->type != VIR_JSON_TYPE_OBJECT)
> (gdb) bt
> 0  in virJSONValueObjectGet
> 1  in virJSONValueObjectGetBoolean
> 2  in qemuMonitorJSONHandleShutdown
> 3  in qemuMonitorJSONIOProcessEvent
> 4  in qemuMonitorJSONIOProcessLine
> 5  in qemuMonitorJSONIOProcess
> 6  in qemuMonitorIOProcess

I think you can truncate is somewhere here.

> 7  in qemuMonitorIO
> 8  in virEventPollDispatchHandles
> 9  in virEventPollRunOnce
> 10 in virEventRunDefaultImpl
> 11 in virNetDaemonRun
> 12 in main
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/qemu/qemu_monitor_json.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 757595dd7..f208dd05a 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -528,7 +528,7 @@ static void qemuMonitorJSONHandleShutdown(qemuMonitorPtr 
> mon, virJSONValuePtr da
>  bool guest = false;
>  virTristateBool guest_initiated = VIR_TRISTATE_BOOL_ABSENT;
>  
> -if (virJSONValueObjectGetBoolean(data, "guest", ) == 0)
> +if (data && virJSONValueObjectGetBoolean(data, "guest", ) == 0)
>  guest_initiated = guest ? VIR_TRISTATE_BOOL_YES : 
> VIR_TRISTATE_BOOL_NO;
>  
>  qemuMonitorEmitShutdown(mon, guest_initiated);

ACK, safe for freeze



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

[libvirt] [PATCH] qemu:json: Fix daemon crash on handling domain shutdown event

[Erik Skultety]

commit a8eba5036 added further checking of the guest shutdown cause, but
this enhancement is available since qemu 2.10, causing a crash because
of a NULL pointer dereference on older qemus.

Thread 1 "libvirtd" received signal SIGSEGV, Segmentation fault.
0x772441af in virJSONValueObjectGet (object=0x0,
 key=0x7fffd5ef11bf "guest")
at util/virjson.c:769
769 if (object->type != VIR_JSON_TYPE_OBJECT)
(gdb) bt
0  in virJSONValueObjectGet
1  in virJSONValueObjectGetBoolean
2  in qemuMonitorJSONHandleShutdown
3  in qemuMonitorJSONIOProcessEvent
4  in qemuMonitorJSONIOProcessLine
5  in qemuMonitorJSONIOProcess
6  in qemuMonitorIOProcess
7  in qemuMonitorIO
8  in virEventPollDispatchHandles
9  in virEventPollRunOnce
10 in virEventRunDefaultImpl
11 in virNetDaemonRun
12 in main

Signed-off-by: Erik Skultety 
---
 src/qemu/qemu_monitor_json.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 757595dd7..f208dd05a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -528,7 +528,7 @@ static void qemuMonitorJSONHandleShutdown(qemuMonitorPtr 
mon, virJSONValuePtr da
 bool guest = false;
 virTristateBool guest_initiated = VIR_TRISTATE_BOOL_ABSENT;
 
-if (virJSONValueObjectGetBoolean(data, "guest", ) == 0)
+if (data && virJSONValueObjectGetBoolean(data, "guest", ) == 0)
 guest_initiated = guest ? VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
 
 qemuMonitorEmitShutdown(mon, guest_initiated);
-- 
2.13.0

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

Re: [libvirt] [PATCH v3] Regression: Report correct CPUs present on executing virsh nodecpumap

[Peter Krempa]

On Thu, May 25, 2017 at 15:47:43 +0530, Nitesh Konkar wrote:
> On executing the virsh nodecpumap command, the number
> of CPUs present was shown as (last cpu online id + 1). This
> patch fixes the issue by reporting the current number of
> CPUs present.

Both the summary and commit message mention virsh, while the error is in
the API.

The summary also is not in the format used by libvirt.

Also you are claiming regression but not pointing to the commit that
broke it.

It was caused by commit c67e04e25fa58104e0fae41f5b874a8067557073.

I'll push this in a while after fix the commit message.


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

Re: [libvirt] [PATCH v2] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND

[Peter Krempa]

On Mon, May 29, 2017 at 14:08:53 -0400, Daniel Liu wrote:
> The option allows someone to run domain-to-native on already existing
>  domain without the need of supplying their XML.  It is basically
>  wrapper around 'virsh dumpxml  | virsh domxml-to-native /dev/stdin'.
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
> ---
>  tools/virsh-domain.c | 52 
> 
>  1 file changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index ccb514ef9..929f9c896 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c

[...]

> @@ -9859,30 +9863,54 @@ static const vshCmdOptDef opts_domxmltonative[] = {
>  static bool
>  cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
>  {
> -bool ret = true;
> +bool ret = false;
>  const char *format = NULL;
> -const char *xmlFile = NULL;
> -char *configData;
> -char *xmlData;
> +const char *domain = NULL;
> +const char *xml = NULL;
> +char *xmlData = NULL;
> +char *configData = NULL;
>  unsigned int flags = 0;
>  virshControlPtr priv = ctl->privData;
> +virDomainPtr dom = NULL;
>  
>  if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> +vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
> +vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)

You don't need this variable after the check below, so it's pointless to
extract it.

>  return false;
>  
> -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);

This macro is documented to work only on booleans, please don't use it
on pointers.

> +
> +if (domain) {
> +if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> +return false;
> +}
> +
> +if (dom) {
> +xmlData = virDomainGetXMLDesc(dom, flags);
> +} else if (xml) {
> +if (virFileReadAll(xml, VSH_MAX_XML_FILE, ) < 0)
> +goto cleanup;
> +}
> +
> +if (!xmlData) {
> +vshError(ctl, "%s",
> + _("need either domain (ID, UUID, or name) or domain XML 
> configuration file path"));

This line is very long.

>  return false;

You'll leak 'dom' here if it was looked up, but getting of the XML
failed.

Also the message is kind of wrong in that case, since you've provided a
valid name, but the XML could not be requested

> +}
>  
> -configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, 
> flags);
> -if (configData != NULL) {
> -vshPrint(ctl, "%s", configData);
> -VIR_FREE(configData);
> +if (!(configData = virConnectDomainXMLToNative(priv->conn, format, 
> xmlData, flags))) {
> +vshError(ctl, "%s",
> + _("convert from domain XML to native command failed"));

For 'xen' the output is not really a command, so this message also is
not very good.

> +goto cleanup;
>  } else {
> -ret = false;
> +vshPrint(ctl, "%s", configData);
> +ret = true;


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

Re: [libvirt] [libvirt-sandbox PATCH] mkinitrd: Add missing fscrypto module

[Guido Günther]

On Mon, May 29, 2017 at 11:42:09AM +0200, Cedric Bosdonnat wrote:
> On Sat, 2017-05-27 at 13:04 +0200, Guido Günther wrote:
> > ---
> >  libvirt-sandbox/libvirt-sandbox-builder-machine.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c 
> > b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
> > index bdec490..7204f71 100644
> > --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c
> > +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c
> > @@ -186,6 +186,7 @@ static gchar 
> > *gvir_sandbox_builder_machine_mkinitrd(GVirSandboxConfig *config,
> >  
> >  /* In case ext4 is built as a module, include it and its deps
> >   * for the root mount */
> > +gvir_sandbox_config_initrd_add_module(initrd, "fscrypto.ko");
> >  gvir_sandbox_config_initrd_add_module(initrd, "mbcache.ko");
> >  gvir_sandbox_config_initrd_add_module(initrd, "jbd2.ko");
> >  gvir_sandbox_config_initrd_add_module(initrd, "crc16.ko");
> 
> ACK

Pushed. Thanks
 -- Guido

> 
> --
> Cedric
> 

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

Re: [libvirt] [libvirt-sandbox PATCH] docker: Don't ignore qemu-img errors

[Guido Günther]

On Mon, May 29, 2017 at 11:43:53AM +0200, Cedric Bosdonnat wrote:
> On Sat, 2017-05-27 at 18:30 +0200, Guido Günther wrote:
> > ---
> >  libvirt-sandbox/image/sources/docker.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libvirt-sandbox/image/sources/docker.py 
> > b/libvirt-sandbox/image/sources/docker.py
> > index 43e9c32..aa5675e 100755
> > --- a/libvirt-sandbox/image/sources/docker.py
> > +++ b/libvirt-sandbox/image/sources/docker.py
> > @@ -662,7 +662,7 @@ class DockerSource(base.Source):
> >  cmd.append("-o")
> >  cmd.append("backing_fmt=qcow2,backing_file=%s" % diskfile)
> >  cmd.append(tempfile)
> > -subprocess.call(cmd)
> > +subprocess.check_call(cmd)
> >  return tempfile
> >  
> >  def get_command(self, template, templatedir, userargs):
> 
> ACK

Pushed. Thanks
 -- Guido

> 
> --
> Cedric
> 

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