[libvirt] libvirt-snmp status

[Michael Corcoran]

Hi All,

I'd like to enquire about the status of libvirt-snmp. It hasn't had any
 commits for a couple of years and no longer compiles against recent
versions of net-snmp. As I understand it, upstream net-snmp deprecated
and then removed the C type "U64" as it conflicts with Perl.
Fortunately this is the only issue and the patch required to get
libvirt-snmp building again simply replaces the C type "U64" with the
type "struct counter64" in a few places. I tried to  submit a patch but
was unable to configure authentication/encryption correctly for git
send-email and I'm unwilling to spend more time on it. Is there any way
to accept patches directly from Github (or anywhere else I can push a
commit)?

My second enquiry relates to the org OID used in the LIBVIRT-MIB. It
currently uses a (presumably) placeholder of 12345. I check the OID
register[1] and found an entry for libvirt (see end of email). I am
guessing that registration didn't go through until after libvirt-snmp
maintenance stopped. I'd like to submit a patch to replace the
placeholder with this org id also.

BR,
Michael Corcoran

[1] https://www.iana.org/assignments/enterprise-numbers/enterprise-numb
ers

36957
  libvirt
Daniel Veillard
  daniel

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

Re: [libvirt] [PATCH 02/20] libxl: Fix possible deadlock in libxlDomainMigrateBegin3Params

[Jim Fehlig]

On 03/09/2018 09:47 AM, John Ferlan wrote:

Commit id '45697fe5' added a check for "Domain-0" to generate
an error during libxlDomainMigrateBegin3Params; however, by
returning NULL, the @vm was left locked since libxlDomObjFromDomain
returns a locked @vm.

Signed-off-by: John Ferlan 
---
  src/libxl/libxl_driver.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c3616a86d..b5101626e 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5895,9 +5895,10 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain,
  return NULL;
  
  if (STREQ_NULLABLE(vm->def->name, "Domain-0")) {

-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("Domain-0 cannot be migrated"));
-return NULL;
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Domain-0 cannot be migrated"));
+virObjectUnlock(vm);
+return NULL;
  }
  
  if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) {




Reviewed-by: Jim Fehlig 

Regards,
Jim

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

[libvirt] [PATCH 08/20] openvz: Use virDomainObjListFindBy{UUID|ID}Ref

[John Ferlan]

For openvzDomObjFromDomainLocked and openvzDomainLookupByID
let's return a locked and referenced @vm object so that callers
can then use the common and more consistent virDomainObjEndAPI
in order to handle cleanup rather than needing to know that the
returned object is locked and calling virObjectUnlock.

The LookupByName already returns the ref counted and locked object,
so this will make things more consistent.

Signed-off-by: John Ferlan 
---
 src/openvz/openvz_driver.c | 76 --
 1 file changed, 26 insertions(+), 50 deletions(-)

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index b31bf0714..b0b72b171 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -95,7 +95,7 @@ openvzDomObjFromDomainLocked(struct openvz_driver *driver,
 virDomainObjPtr vm;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
+if (!(vm = virDomainObjListFindByUUIDRef(driver->domains, uuid))) {
 virUUIDFormat(uuid, uuidstr);
 
 virReportError(VIR_ERR_NO_DOMAIN,
@@ -329,8 +329,7 @@ openvzDomainGetHostname(virDomainPtr dom, unsigned int 
flags)
 }
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return hostname;
 
  error:
@@ -347,7 +346,7 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr 
conn,
 virDomainPtr dom = NULL;
 
 openvzDriverLock(driver);
-vm = virDomainObjListFindByID(driver->domains, id);
+vm = virDomainObjListFindByIDRef(driver->domains, id);
 openvzDriverUnlock(driver);
 
 if (!vm) {
@@ -359,8 +358,7 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr 
conn,
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
@@ -391,8 +389,7 @@ static char *openvzDomainGetOSType(virDomainPtr dom)
 
 ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(vm->def->os.type)));
 
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -409,8 +406,7 @@ static virDomainPtr openvzDomainLookupByUUID(virConnectPtr 
conn,
 
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
@@ -469,8 +465,7 @@ static int openvzDomainGetInfo(virDomainPtr dom,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -492,8 +487,7 @@ openvzDomainGetState(virDomainPtr dom,
 
 ret = openvzGetVEStatus(vm, state, reason);
 
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -509,8 +503,7 @@ static int openvzDomainIsActive(virDomainPtr dom)
 
 ret = virDomainObjIsActive(obj);
 
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -526,8 +519,7 @@ static int openvzDomainIsPersistent(virDomainPtr dom)
 
 ret = obj->persistent;
 
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -549,8 +541,7 @@ static char *openvzDomainGetXMLDesc(virDomainPtr dom, 
unsigned int flags) {
 ret = virDomainDefFormat(vm->def, driver->caps,
  virDomainDefFormatConvertXMLFlags(flags));
 
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -600,8 +591,7 @@ static int openvzDomainSuspend(virDomainPtr dom)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -631,8 +621,7 @@ static int openvzDomainResume(virDomainPtr dom)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -670,8 +659,7 @@ openvzDomainShutdownFlags(virDomainPtr dom,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -724,8 +712,7 @@ static int openvzDomainReboot(virDomainPtr dom,
 virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1183,8 +1170,7 @@ openvzDomainUndefineFlags(virDomainPtr dom,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 openvzDriverUnlock(driver);
 return ret;
 }
@@ -1213,8 +1199,7 @@ openvzDomainSetAutostart(virDomainPtr dom, int autostart)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1243,8 +1228,7 @@ openvzDomainGetAutostart(virDomainPtr dom, int *autostart)
  cleanup:
 VIR_FREE(value);
 
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1336,8 +1320,7 

[libvirt] [PATCH 01/20] bhyve: Use virDomainObjListFindBy{UUID|ID}Ref

[John Ferlan]

For bhyveDomObjFromDomain, bhyveDomainLookupByUUID, and
bhyveDomainLookupByID let's return a locked and referenced
@vm object so that callers can then use the common and more
consistent virDomainObjEndAPI in order to handle cleanup rather
than needing to know that the returned object is locked and
calling virObjectUnlock.

The LookupByName already returns the ref counted and locked object,
so this will make things more consistent.

Signed-off-by: John Ferlan 
---
 src/bhyve/bhyve_driver.c | 58 ++--
 1 file changed, 21 insertions(+), 37 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 849d3abcd..79963570c 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -168,7 +168,7 @@ bhyveDomObjFromDomain(virDomainPtr domain)
 bhyveConnPtr privconn = domain->conn->privateData;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+vm = virDomainObjListFindByUUIDRef(privconn->domains, domain->uuid);
 if (!vm) {
 virUUIDFormat(domain->uuid, uuidstr);
 virReportError(VIR_ERR_NO_DOMAIN,
@@ -312,8 +312,7 @@ bhyveDomainGetInfo(virDomainPtr domain, virDomainInfoPtr 
info)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -338,8 +337,7 @@ bhyveDomainGetState(virDomainPtr domain,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -359,8 +357,7 @@ bhyveDomainGetAutostart(virDomainPtr domain, int *autostart)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -423,8 +420,7 @@ bhyveDomainSetAutostart(virDomainPtr domain, int autostart)
  cleanup:
 VIR_FREE(configFile);
 VIR_FREE(autostartLink);
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -443,8 +439,7 @@ bhyveDomainIsActive(virDomainPtr domain)
 ret = virDomainObjIsActive(obj);
 
  cleanup:
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -463,8 +458,7 @@ bhyveDomainIsPersistent(virDomainPtr domain)
 ret = obj->persistent;
 
  cleanup:
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -484,8 +478,7 @@ bhyveDomainGetOSType(virDomainPtr dom)
 goto cleanup;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -512,8 +505,7 @@ bhyveDomainGetXMLDesc(virDomainPtr domain, unsigned int 
flags)
 
 virObjectUnref(caps);
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -630,8 +622,7 @@ bhyveDomainUndefine(virDomainPtr domain)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 if (event)
 virObjectEventStateQueue(privconn->domainEventState, event);
 return ret;
@@ -803,7 +794,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByUUID(privconn->domains, uuid);
+vm = virDomainObjListFindByUUIDRef(privconn->domains, uuid);
 
 if (!vm) {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -819,8 +810,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
@@ -857,7 +847,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByID(privconn->domains, id);
+vm = virDomainObjListFindByIDRef(privconn->domains, id);
 
 if (!vm) {
 virReportError(VIR_ERR_NO_DOMAIN,
@@ -871,8 +861,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
@@ -913,8 +902,7 @@ bhyveDomainCreateWithFlags(virDomainPtr dom,
   
VIR_DOMAIN_EVENT_STARTED_BOOTED);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 if (event)
 virObjectEventStateQueue(privconn->domainEventState, event);
 return ret;
@@ -1028,8 +1016,7 @@ bhyveDomainDestroy(virDomainPtr dom)
 }
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 if (event)
 virObjectEventStateQueue(privconn->domainEventState, event);
 return ret;
@@ -1056,8 +1043,7 @@ bhyveDomainShutdown(virDomainPtr dom)
 ret = virBhyveProcessShutdown(vm);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1100,8 

[libvirt] [PATCH 10/20] uml: Add more specific error message on failed FindBy call

[John Ferlan]

Rather than an empty failed to find, let's provide a bit more
knowledge about what we failed to find by using the name string
or the id value.

Signed-off-by: John Ferlan 
---
 src/uml/uml_driver.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index ea8fca38b..d10a9ba62 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1385,7 +1385,8 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr 
conn,
 umlDriverUnlock(driver);
 
 if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching id '%d'"), id);
 goto cleanup;
 }
 
@@ -1433,7 +1434,8 @@ static virDomainPtr umlDomainLookupByName(virConnectPtr 
conn,
 umlDriverUnlock(driver);
 
 if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching name '%s'"), name);
 goto cleanup;
 }
 
-- 
2.13.6

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

[libvirt] [PATCH 02/20] libxl: Fix possible deadlock in libxlDomainMigrateBegin3Params

[John Ferlan]

Commit id '45697fe5' added a check for "Domain-0" to generate
an error during libxlDomainMigrateBegin3Params; however, by
returning NULL, the @vm was left locked since libxlDomObjFromDomain
returns a locked @vm.

Signed-off-by: John Ferlan 
---
 src/libxl/libxl_driver.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c3616a86d..b5101626e 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5895,9 +5895,10 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain,
 return NULL;
 
 if (STREQ_NULLABLE(vm->def->name, "Domain-0")) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("Domain-0 cannot be migrated"));
-return NULL;
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Domain-0 cannot be migrated"));
+virObjectUnlock(vm);
+return NULL;
 }
 
 if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) {
-- 
2.13.6

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

[libvirt] [PATCH 07/20] openvz: Add more descriptive error message on Find failure

[John Ferlan]

If openvzDomainLookupByID or openvzDomainLookupByName fails
to find a vm, let's be a bit more descriptive by providing
the failing id or name in the error message.

Signed-off-by: John Ferlan 
---
 src/openvz/openvz_driver.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 167ba2f7a..b31bf0714 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -351,7 +351,8 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr 
conn,
 openvzDriverUnlock(driver);
 
 if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching id '%d'"), id);
 goto cleanup;
 }
 
@@ -425,7 +426,8 @@ static virDomainPtr openvzDomainLookupByName(virConnectPtr 
conn,
 openvzDriverUnlock(driver);
 
 if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching name '%s'"), name);
 goto cleanup;
 }
 
@@ -1113,8 +1115,8 @@ openvzDomainCreateWithFlags(virDomainPtr dom, unsigned 
int flags)
 openvzDriverUnlock(driver);
 
 if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, "%s",
-   _("no domain with matching id"));
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching name '%s'"), dom->name);
 goto cleanup;
 }
 
-- 
2.13.6

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

[libvirt] [PATCH 16/20] vz: Unify vzDomObjFromDomain{Ref}

[John Ferlan]

Rather than have two API's doing different things for different
callers, let's make one API that will always return a locked and
ref counted object. That way, the callers will always know that
they must call virDomainObjEndAPI and not have to decide whether
they should call virObjectUnlock instead.

This will make things consistent with LookupByName which returns
the locked and ref counted object.

Signed-off-by: John Ferlan 
---
 src/vz/vz_driver.c | 111 ++---
 src/vz/vz_utils.c  |  32 +--
 src/vz/vz_utils.h  |   1 -
 3 files changed, 56 insertions(+), 88 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 6d02ef274..1118ef92f 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -654,7 +654,7 @@ vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
 vzDomObjPtr privdom;
 int ret = -1;
 
-if (!(dom = vzDomObjFromDomainRef(domain)))
+if (!(dom = vzDomObjFromDomain(domain)))
 goto cleanup;
 
 if (virDomainGetInfoEnsureACL(domain->conn, dom->def) < 0)
@@ -703,7 +703,7 @@ vzDomainGetOSType(virDomainPtr domain)
 ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(dom->def->os.type)));
 
  cleanup:
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -722,7 +722,7 @@ vzDomainIsPersistent(virDomainPtr domain)
 ret = 1;
 
  cleanup:
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -745,7 +745,7 @@ vzDomainGetState(virDomainPtr domain,
 ret = 0;
 
  cleanup:
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -771,7 +771,7 @@ vzDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
 ret = virDomainDefFormat(def, privconn->driver->caps, flags);
 
  cleanup:
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -791,7 +791,7 @@ vzDomainGetAutostart(virDomainPtr domain, int *autostart)
 ret = 0;
 
  cleanup:
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -985,7 +985,7 @@ vzDomainGetVcpus(virDomainPtr domain,
 size_t i;
 int ret = -1;
 
-if (!(dom = vzDomObjFromDomainRef(domain)))
+if (!(dom = vzDomObjFromDomain(domain)))
 return -1;
 
 if (virDomainGetVcpusEnsureACL(domain->conn, dom->def) < 0)
@@ -1087,7 +1087,7 @@ vzDomainSuspend(virDomainPtr domain)
 int ret = -1;
 bool job = false;
 
-if (!(dom = vzDomObjFromDomainRef(domain)))
+if (!(dom = vzDomObjFromDomain(domain)))
 return -1;
 
 if (virDomainSuspendEnsureACL(domain->conn, dom->def) < 0)
@@ -1124,7 +1124,7 @@ vzDomainResume(virDomainPtr domain)
 int ret = -1;
 bool job = false;
 
-if (!(dom = vzDomObjFromDomainRef(domain)))
+if (!(dom = vzDomObjFromDomain(domain)))
 return -1;
 
 if (virDomainResumeEnsureACL(domain->conn, dom->def) < 0)
@@ -1163,7 +1163,7 @@ vzDomainCreateWithFlags(virDomainPtr domain, unsigned int 
flags)
 
 virCheckFlags(0, -1);
 
-if (!(dom = vzDomObjFromDomainRef(domain)))
+if (!(dom = vzDomObjFromDomain(domain)))
 return -1;
 
 if (virDomainCreateWithFlagsEnsureACL(domain->conn, dom->def) < 0)
@@ -1202,7 +1202,7 @@ vzDomainDestroyFlags(virDomainPtr domain, unsigned int 
flags)
 
 virCheckFlags(0, -1);
 
-if (!(dom = vzDomObjFromDomainRef(domain)))
+if (!(dom = vzDomObjFromDomain(domain)))
 return -1;
 
 if (virDomainDestroyFlagsEnsureACL(domain->conn, dom->def) < 0)
@@ -1247,7 +1247,7 @@ vzDomainShutdownFlags(virDomainPtr domain, unsigned int 
flags)
 
 virCheckFlags(0, -1);
 
-if (!(dom = vzDomObjFromDomainRef(domain)))
+if (!(dom = vzDomObjFromDomain(domain)))
 return -1;
 
 if (virDomainShutdownFlagsEnsureACL(domain->conn, dom->def, flags) < 0)
@@ -1291,7 +1291,7 @@ vzDomainReboot(virDomainPtr domain, unsigned int flags)
 
 virCheckFlags(0, -1);
 
-if (!(dom = vzDomObjFromDomainRef(domain)))
+if (!(dom = vzDomObjFromDomain(domain)))
 return -1;
 
 if (virDomainRebootEnsureACL(domain->conn, dom->def, flags) < 0)
@@ -1334,7 +1334,7 @@ static int vzDomainIsActive(virDomainPtr domain)
 ret = virDomainObjIsActive(dom);
 
  cleanup:
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 
 return ret;
 }
@@ -1357,7 +1357,7 @@ vzDomainUndefineFlags(virDomainPtr domain,
 virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
   VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
 
-if (!(dom = vzDomObjFromDomainRef(domain)))
+if (!(dom = vzDomObjFromDomain(domain)))
 return -1;
 
 if (virDomainUndefineFlagsEnsureACL(domain->conn, dom->def) < 0)
@@ -1409,7 +1409,7 @@ vzDomainHasManagedSaveImage(virDomainPtr domain, unsigned 
int flags)
 ret = 0;
 
  cleanup:
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 
 return ret;
 }
@@ -1426,7 +1426,7 @@ vzDomainManagedSave(virDomainPtr domain, unsigned int 
flags)
 

[libvirt] [PATCH 19/20] conf: Rework/rename virDomainObjListFindByUUIDRef

[John Ferlan]

Now that every caller is using virDomainObjListFindByUUIDRef,
let's just remove it and keep the name as virDomainObjListFindByUUID.

That allows reworking the virdomainobjlist API's a bit to be more
like virDomainObjListFindByName.

Signed-off-by: John Ferlan 
---
 src/bhyve/bhyve_driver.c |  4 ++--
 src/conf/virdomainobjlist.c  | 35 +++
 src/conf/virdomainobjlist.h  |  2 --
 src/libvirt_private.syms |  1 -
 src/libxl/libxl_driver.c |  4 ++--
 src/lxc/lxc_driver.c |  4 ++--
 src/openvz/openvz_driver.c   |  2 +-
 src/qemu/qemu_driver.c   |  4 ++--
 src/test/test_driver.c   |  4 ++--
 src/uml/uml_driver.c |  4 ++--
 src/util/virclosecallbacks.c |  4 ++--
 src/vmware/vmware_driver.c   |  2 +-
 src/vz/vz_driver.c   |  4 ++--
 src/vz/vz_sdk.c  | 14 +++---
 src/vz/vz_utils.c|  2 +-
 15 files changed, 33 insertions(+), 57 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 79963570c..4f95f6c15 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -168,7 +168,7 @@ bhyveDomObjFromDomain(virDomainPtr domain)
 bhyveConnPtr privconn = domain->conn->privateData;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-vm = virDomainObjListFindByUUIDRef(privconn->domains, domain->uuid);
+vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
 if (!vm) {
 virUUIDFormat(domain->uuid, uuidstr);
 virReportError(VIR_ERR_NO_DOMAIN,
@@ -794,7 +794,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByUUIDRef(privconn->domains, uuid);
+vm = virDomainObjListFindByUUID(privconn->domains, uuid);
 
 if (!vm) {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 87a742b1e..3290dfa29 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -152,10 +152,10 @@ virDomainObjListFindByIDRef(virDomainObjListPtr doms,
 return virDomainObjListFindByIDInternal(doms, id, true);
 }
 
-static virDomainObjPtr
-virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms,
-   const unsigned char *uuid,
-   bool ref)
+
+virDomainObjPtr
+virDomainObjListFindByUUID(virDomainObjListPtr doms,
+   const unsigned char *uuid)
 {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 virDomainObjPtr obj;
@@ -164,41 +164,20 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr 
doms,
 virUUIDFormat(uuid, uuidstr);
 
 obj = virHashLookup(doms->objs, uuidstr);
-if (ref) {
-virObjectRef(obj);
-virObjectRWUnlock(doms);
-}
+virObjectRef(obj);
+virObjectRWUnlock(doms);
 if (obj) {
 virObjectLock(obj);
 if (obj->removing) {
 virObjectUnlock(obj);
-if (ref)
-virObjectUnref(obj);
+virObjectUnref(obj);
 obj = NULL;
 }
 }
-if (!ref)
-virObjectRWUnlock(doms);
 return obj;
 }
 
 
-virDomainObjPtr
-virDomainObjListFindByUUID(virDomainObjListPtr doms,
-   const unsigned char *uuid)
-{
-return virDomainObjListFindByUUIDInternal(doms, uuid, false);
-}
-
-
-virDomainObjPtr
-virDomainObjListFindByUUIDRef(virDomainObjListPtr doms,
-  const unsigned char *uuid)
-{
-return virDomainObjListFindByUUIDInternal(doms, uuid, true);
-}
-
-
 virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
const char *name)
 {
diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
index bb186bde3..1b77a95ba 100644
--- a/src/conf/virdomainobjlist.h
+++ b/src/conf/virdomainobjlist.h
@@ -38,8 +38,6 @@ virDomainObjPtr 
virDomainObjListFindByIDRef(virDomainObjListPtr doms,
 int id);
 virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
const unsigned char *uuid);
-virDomainObjPtr virDomainObjListFindByUUIDRef(virDomainObjListPtr doms,
-  const unsigned char *uuid);
 virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
const char *name);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3766e20d3..6f0cd9680 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -951,7 +951,6 @@ virDomainObjListFindByID;
 virDomainObjListFindByIDRef;
 virDomainObjListFindByName;
 virDomainObjListFindByUUID;
-virDomainObjListFindByUUIDRef;
 virDomainObjListForEach;
 virDomainObjListGetActiveIDs;
 virDomainObjListGetInactiveNames;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e78fe2d4b..1379e9b83 

[libvirt] [PATCH 14/20] vmware: Properly clean up in vmwareDomainLookupByName

[John Ferlan]

The virDomainObjListFindByName returns a locked and reffed
domain object, all we did was unlock it, leaving an extra
ref. Use the virDomainObjEndAPI to cleanup instead.

Signed-off-by: John Ferlan 
---
 src/vmware/vmware_driver.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index 9cd0bc438..d17fdfe3b 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -909,8 +909,7 @@ vmwareDomainLookupByName(virConnectPtr conn, const char 
*name)
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
-- 
2.13.6

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

[libvirt] [PATCH 18/20] test: Use virDomainObjListFindByUUIDRef

[John Ferlan]

Rather than using virDomainObjListFindByUUID, use the ref
counting API in order to use virDomainObjEndAPI instead of
virObjectUnlock (for common look and feel).

Signed-off-by: John Ferlan 
---
 src/test/test_driver.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 043caa976..ce8c1001d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1725,7 +1725,7 @@ static virDomainPtr testDomainLookupByUUID(virConnectPtr 
conn,
 virDomainPtr ret = NULL;
 virDomainObjPtr dom;
 
-if (!(dom = virDomainObjListFindByUUID(privconn->domains, uuid))) {
+if (!(dom = virDomainObjListFindByUUIDRef(privconn->domains, uuid))) {
 virReportError(VIR_ERR_NO_DOMAIN, NULL);
 goto cleanup;
 }
@@ -1733,8 +1733,7 @@ static virDomainPtr testDomainLookupByUUID(virConnectPtr 
conn,
 ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id);
 
  cleanup:
-if (dom)
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 return ret;
 }
 
-- 
2.13.6

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

[libvirt] [PATCH 17/20] vz: Use virDomainObjListFindBy{UUID|ID}Ref

[John Ferlan]

For vzDomainLookupByID and vzDomainLookupByUUID let's
return a locked and referenced @vm object so that callers
can then use the common and more consistent virDomainObjEndAPI
in order to handle cleanup rather than needing to know that the
returned object is locked and calling virObjectUnlock.

The LookupByName already returns the ref counted and locked object,
so this will make things more consistent.

Also adjust the prlsdkHandle{VmState|VmRemoved|Perf}Event APIs
in the same manner.

Signed-off-by: John Ferlan 
---
 src/vz/vz_driver.c |  8 
 src/vz/vz_sdk.c| 15 ---
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 1118ef92f..68ae2f8e0 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -578,7 +578,7 @@ vzDomainLookupByID(virConnectPtr conn, int id)
 virDomainPtr ret = NULL;
 virDomainObjPtr dom;
 
-dom = virDomainObjListFindByID(privconn->driver->domains, id);
+dom = virDomainObjListFindByIDRef(privconn->driver->domains, id);
 
 if (dom == NULL) {
 virReportError(VIR_ERR_NO_DOMAIN, NULL);
@@ -591,7 +591,7 @@ vzDomainLookupByID(virConnectPtr conn, int id)
 ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id);
 
  cleanup:
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -602,7 +602,7 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned 
char *uuid)
 virDomainPtr ret = NULL;
 virDomainObjPtr dom;
 
-dom = virDomainObjListFindByUUID(privconn->driver->domains, uuid);
+dom = virDomainObjListFindByUUIDRef(privconn->driver->domains, uuid);
 
 if (dom == NULL) {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -618,7 +618,7 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned 
char *uuid)
 ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id);
 
  cleanup:
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 return ret;
 }
 
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index a5b9f2da6..b8f13f88a 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2144,7 +2144,7 @@ prlsdkHandleVmStateEvent(vzDriverPtr driver,
 virDomainEventType lvEventType = 0;
 int lvEventTypeDetails = 0;
 
-dom = virDomainObjListFindByUUID(driver->domains, uuid);
+dom = virDomainObjListFindByUUIDRef(driver->domains, uuid);
 if (dom == NULL)
 return;
 
@@ -2166,7 +2166,7 @@ prlsdkHandleVmStateEvent(vzDriverPtr driver,
 
  cleanup:
 PrlHandle_Free(eventParam);
-virObjectUnlock(dom);
+virObjectEndAPI();
 return;
 }
 
@@ -2225,7 +2225,7 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
 {
 virDomainObjPtr dom = NULL;
 
-dom = virDomainObjListFindByUUID(driver->domains, uuid);
+dom = virDomainObjListFindByUUIDRef(driver->domains, uuid);
 /* domain was removed from the list from the libvirt
  * API function in current connection */
 if (dom == NULL)
@@ -2235,6 +2235,7 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
 VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
 
 virDomainObjListRemove(driver->domains, dom);
+virDomainObjEndAPI();
 return;
 }
 
@@ -2246,7 +2247,7 @@ prlsdkHandlePerfEvent(vzDriverPtr driver,
 virDomainObjPtr dom = NULL;
 vzDomObjPtr privdom = NULL;
 
-if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid))) {
+if (!(dom = virDomainObjListFindByUUIDRef(driver->domains, uuid))) {
 PrlHandle_Free(event);
 return;
 }
@@ -2255,7 +2256,7 @@ prlsdkHandlePerfEvent(vzDriverPtr driver,
 PrlHandle_Free(privdom->stats);
 privdom->stats = event;
 
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 }
 
 static void
@@ -2269,7 +2270,7 @@ prlsdkHandleMigrationProgress(vzDriverPtr driver,
 PRL_HANDLE param = PRL_INVALID_HANDLE;
 PRL_RESULT pret;
 
-if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid)))
+if (!(dom = virDomainObjListFindByUUIDRef(driver->domains, uuid)))
 return;
 
 pret = PrlEvent_GetParam(event, 0, );
@@ -2283,7 +2284,7 @@ prlsdkHandleMigrationProgress(vzDriverPtr driver,
 
  cleanup:
 PrlHandle_Free(param);
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 }
 
 static PRL_RESULT
-- 
2.13.6

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

[libvirt] [PATCH 15/20] vmware: Use virDomainObjListFindBy{UUID|ID}Ref

[John Ferlan]

For vmwareDomObjFromDomainLocked and vmwareDomainLookupByID
let's return a locked and referenced @vm object so that callers
can then use the common and more consistent virDomainObjEndAPI
in order to handle cleanup rather than needing to know that the
returned object is locked and calling virObjectUnlock.

The LookupByName already returns the ref counted and locked object,
so this will make things more consistent.

Signed-off-by: John Ferlan 
---
 src/vmware/vmware_driver.c | 49 --
 1 file changed, 17 insertions(+), 32 deletions(-)

diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index d17fdfe3b..37a7b1932 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -67,7 +67,7 @@ vmwareDomObjFromDomainLocked(struct vmware_driver *driver,
 virDomainObjPtr vm;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
+if (!(vm = virDomainObjListFindByUUIDRef(driver->domains, uuid))) {
 virUUIDFormat(uuid, uuidstr);
 
 virReportError(VIR_ERR_NO_DOMAIN,
@@ -515,8 +515,7 @@ vmwareDomainShutdownFlags(virDomainPtr dom,
 
 ret = 0;
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 vmwareDriverUnlock(driver);
 return ret;
 }
@@ -577,8 +576,7 @@ vmwareDomainSuspend(virDomainPtr dom)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -619,8 +617,7 @@ vmwareDomainResume(virDomainPtr dom)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -660,8 +657,7 @@ vmwareDomainReboot(virDomainPtr dom, unsigned int flags)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -772,8 +768,7 @@ vmwareDomainCreateWithFlags(virDomainPtr dom,
 ret = vmwareStartVM(driver, vm);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 vmwareDriverUnlock(driver);
 return ret;
 }
@@ -817,8 +812,7 @@ vmwareDomainUndefineFlags(virDomainPtr dom,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 vmwareDriverUnlock(driver);
 return ret;
 }
@@ -837,7 +831,7 @@ vmwareDomainLookupByID(virConnectPtr conn, int id)
 virDomainPtr dom = NULL;
 
 vmwareDriverLock(driver);
-vm = virDomainObjListFindByID(driver->domains, id);
+vm = virDomainObjListFindByIDRef(driver->domains, id);
 vmwareDriverUnlock(driver);
 
 if (!vm) {
@@ -849,8 +843,7 @@ vmwareDomainLookupByID(virConnectPtr conn, int id)
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
@@ -866,8 +859,7 @@ vmwareDomainGetOSType(virDomainPtr dom)
 
 ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(vm->def->os.type)));
 
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -884,8 +876,7 @@ vmwareDomainLookupByUUID(virConnectPtr conn, const unsigned 
char *uuid)
 
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
@@ -925,8 +916,7 @@ vmwareDomainIsActive(virDomainPtr dom)
 
 ret = virDomainObjIsActive(obj);
 
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -943,8 +933,7 @@ vmwareDomainIsPersistent(virDomainPtr dom)
 
 ret = obj->persistent;
 
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -964,8 +953,7 @@ vmwareDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
 ret = virDomainDefFormat(vm->def, driver->caps,
  virDomainDefFormatConvertXMLFlags(flags));
 
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1097,8 +1085,7 @@ vmwareDomainGetInfo(virDomainPtr dom, virDomainInfoPtr 
info)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1124,8 +,7 @@ vmwareDomainGetState(virDomainPtr dom,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1167,8 +1153,7 @@ vmwareDomainHasManagedSaveImage(virDomainPtr dom, 
unsigned int flags)
 
 ret = 0;
 
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
-- 
2.13.6

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

[libvirt] [PATCH 12/20] vmware: Create accessors to virDomainObjListFindByUUID

[John Ferlan]

Rather than repeat code throughout, create and use a couple of
accessors in order to lookup by UUID.

Signed-off-by: John Ferlan 
---
 src/vmware/vmware_driver.c | 180 +++--
 1 file changed, 61 insertions(+), 119 deletions(-)

diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index 8b487c4a7..121751e27 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -59,6 +59,39 @@ vmwareDriverUnlock(struct vmware_driver *driver)
 virMutexUnlock(>lock);
 }
 
+
+static virDomainObjPtr
+vmwareDomObjFromDomainLocked(struct vmware_driver *driver,
+ const unsigned char *uuid)
+{
+virDomainObjPtr vm;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
+virUUIDFormat(uuid, uuidstr);
+
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching uuid '%s'"), uuidstr);
+return NULL;
+}
+
+return vm;
+}
+
+
+static virDomainObjPtr
+vmwareDomObjFromDomain(struct vmware_driver *driver,
+   const unsigned char *uuid)
+{
+virDomainObjPtr vm;
+
+vmwareDriverLock(driver);
+vm = vmwareDomObjFromDomainLocked(driver, uuid);
+vmwareDriverUnlock(driver);
+return vm;
+}
+
+
 static void *
 vmwareDataAllocFunc(void *opaque ATTRIBUTE_UNUSED)
 {
@@ -460,13 +493,8 @@ vmwareDomainShutdownFlags(virDomainPtr dom,
 
 vmwareDriverLock(driver);
 
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, "%s",
-   _("no domain with matching uuid"));
+if (!(vm = vmwareDomObjFromDomainLocked(driver, dom->uuid)))
 goto cleanup;
-}
 
 if (vmwareUpdateVMStatus(driver, vm) < 0)
 goto cleanup;
@@ -531,15 +559,8 @@ vmwareDomainSuspend(virDomainPtr dom)
 return ret;
 }
 
-vmwareDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-vmwareDriverUnlock(driver);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, "%s",
-   _("no domain with matching uuid"));
-goto cleanup;
-}
+if (!(vm = vmwareDomObjFromDomain(driver, dom->uuid)))
+return -1;
 
 vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type));
 vmwareSetSentinal(cmd, ((vmwareDomainPtr) vm->privateData)->vmxPath);
@@ -580,15 +601,8 @@ vmwareDomainResume(virDomainPtr dom)
 return ret;
 }
 
-vmwareDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-vmwareDriverUnlock(driver);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, "%s",
-   _("no domain with matching uuid"));
-goto cleanup;
-}
+if (!(vm = vmwareDomObjFromDomain(driver, dom->uuid)))
+return -1;
 
 vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type));
 vmwareSetSentinal(cmd, ((vmwareDomainPtr) vm->privateData)->vmxPath);
@@ -624,15 +638,8 @@ vmwareDomainReboot(virDomainPtr dom, unsigned int flags)
 
 virCheckFlags(0, -1);
 
-vmwareDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-vmwareDriverUnlock(driver);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, "%s",
-   _("no domain with matching uuid"));
-goto cleanup;
-}
+if (!(vm = vmwareDomObjFromDomain(driver, dom->uuid)))
+return -1;
 
 vmxPath = ((vmwareDomainPtr) vm->privateData)->vmxPath;
 vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type));
@@ -750,14 +757,8 @@ vmwareDomainCreateWithFlags(virDomainPtr dom,
 virCheckFlags(0, -1);
 
 vmwareDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-if (!vm) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(dom->uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
-   _("No domain with matching uuid '%s'"), uuidstr);
+if (!(vm = vmwareDomObjFromDomainLocked(driver, dom->uuid)))
 goto cleanup;
-}
 
 if (vmwareUpdateVMStatus(driver, vm) < 0)
 goto cleanup;
@@ -794,16 +795,8 @@ vmwareDomainUndefineFlags(virDomainPtr dom,
 virCheckFlags(0, -1);
 
 vmwareDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-
-if (!vm) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-
-virUUIDFormat(dom->uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
-   _("no domain with matching uuid '%s'"), uuidstr);
+if (!(vm = vmwareDomObjFromDomainLocked(driver, dom->uuid)))
 goto cleanup;
-}
 
 if (!vm->persistent) {
 virReportError(VIR_ERR_OPERATION_INVALID,
@@ -867,18 +860,11 @@ vmwareDomainGetOSType(virDomainPtr dom)
 virDomainObjPtr vm;
 char *ret = NULL;
 
-

[libvirt] [PATCH 11/20] uml: Use virDomainObjListFindBy{UUID|ID}Ref

[John Ferlan]

For umlDomObjFromDomainLocked and umlDomainLookupByID let's
return a locked and referenced @vm object so that callers
can then use the common and more consistent virDomainObjEndAPI
in order to handle cleanup rather than needing to know that the
returned object is locked and calling virObjectUnlock.

The LookupByName already returns the ref counted and locked object,
so this will make things more consistent.

Signed-off-by: John Ferlan 
---
 src/uml/uml_driver.c | 81 ++--
 1 file changed, 28 insertions(+), 53 deletions(-)

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index d10a9ba62..41c607e66 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -171,7 +171,7 @@ umlDomObjFromDomainLocked(struct uml_driver *driver,
 virDomainObjPtr vm;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
+if (!(vm = virDomainObjListFindByUUIDRef(driver->domains, uuid))) {
 virUUIDFormat(uuid, uuidstr);
 
 virReportError(VIR_ERR_NO_DOMAIN,
@@ -767,8 +767,7 @@ static int umlProcessAutoDestroyDom(void *payload,
 return 0;
 }
 
-if (!(dom = virDomainObjListFindByUUID(data->driver->domains,
-   uuid))) {
+if (!(dom = virDomainObjListFindByUUIDRef(data->driver->domains, uuid))) {
 VIR_DEBUG("No domain object to kill");
 return 0;
 }
@@ -780,11 +779,10 @@ static int umlProcessAutoDestroyDom(void *payload,
  VIR_DOMAIN_EVENT_STOPPED,
  VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
 
-if (dom && !dom->persistent)
+if (!dom->persistent)
 virDomainObjListRemove(data->driver->domains, dom);
 
-if (dom)
-virObjectUnlock(dom);
+virDomainObjEndAPI();
 if (event)
 umlDomainEventQueue(data->driver, event);
 virHashRemoveEntry(data->driver->autodestroy, uuidstr);
@@ -1381,7 +1379,7 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr 
conn,
 virDomainPtr dom = NULL;
 
 umlDriverLock(driver);
-vm = virDomainObjListFindByID(driver->domains, id);
+vm = virDomainObjListFindByIDRef(driver->domains, id);
 umlDriverUnlock(driver);
 
 if (!vm) {
@@ -1396,8 +1394,7 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr 
conn,
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
@@ -1417,8 +1414,7 @@ static virDomainPtr umlDomainLookupByUUID(virConnectPtr 
conn,
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
@@ -1465,8 +1461,7 @@ static int umlDomainIsActive(virDomainPtr dom)
 ret = virDomainObjIsActive(obj);
 
  cleanup:
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1486,8 +1481,7 @@ static int umlDomainIsPersistent(virDomainPtr dom)
 ret = obj->persistent;
 
  cleanup:
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1506,8 +1500,7 @@ static int umlDomainIsUpdated(virDomainPtr dom)
 ret = obj->updated;
 
  cleanup:
-if (obj)
-virObjectUnlock(obj);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1668,8 +1661,7 @@ static int umlDomainShutdownFlags(virDomainPtr dom,
 
  cleanup:
 VIR_FREE(info);
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1710,8 +1702,7 @@ umlDomainDestroyFlags(virDomainPtr dom,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 if (event)
 umlDomainEventQueue(driver, event);
 umlDriverUnlock(driver);
@@ -1740,8 +1731,7 @@ static char *umlDomainGetOSType(virDomainPtr dom) {
 goto cleanup;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return type;
 }
 
@@ -1762,8 +1752,7 @@ umlDomainGetMaxMemory(virDomainPtr dom)
 ret = virDomainDefGetMemoryTotal(vm->def);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1789,8 +1778,7 @@ static int umlDomainSetMaxMemory(virDomainPtr dom, 
unsigned long newmax)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1822,8 +1810,7 @@ static int umlDomainSetMemory(virDomainPtr dom, unsigned 
long newmem)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1858,8 +1845,7 @@ static int umlDomainGetInfo(virDomainPtr dom,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1886,8 +1872,7 @@ 

[libvirt] [PATCH 20/20] conf: Rework/rename virDomainObjListFindByIDRef

[John Ferlan]

Rework the code such that virDomainObjListFindByID will always
return a locked/ref counted object so that the callers can
always do the same cleanup logic to call virDomainObjEndAPI.
Makes accessing the objects much more consistent.

NB:
There were 3 callers (lxcDomainLookupByID, testDomainLookupByID,
and qemuDomainLookupByID) that were already using the ByID name,
but not the virDomainObjEndAPI - these were changed as well in
this update/patch.

Signed-off-by: John Ferlan 
---
 src/bhyve/bhyve_driver.c|  2 +-
 src/conf/virdomainobjlist.c | 35 +--
 src/conf/virdomainobjlist.h |  2 --
 src/libvirt_private.syms|  1 -
 src/libxl/libxl_domain.c|  2 +-
 src/libxl/libxl_driver.c|  2 +-
 src/lxc/lxc_driver.c|  3 +--
 src/openvz/openvz_driver.c  |  2 +-
 src/qemu/qemu_driver.c  |  5 ++---
 src/test/test_driver.c  |  3 +--
 src/uml/uml_driver.c|  2 +-
 src/vmware/vmware_driver.c  |  2 +-
 src/vz/vz_driver.c  |  2 +-
 13 files changed, 20 insertions(+), 43 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 4f95f6c15..1fd31912d 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -847,7 +847,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByIDRef(privconn->domains, id);
+vm = virDomainObjListFindByID(privconn->domains, id);
 
 if (!vm) {
 virReportError(VIR_ERR_NO_DOMAIN,
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 3290dfa29..983b6fda7 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -112,44 +112,27 @@ static int virDomainObjListSearchID(const void *payload,
 return want;
 }
 
-static virDomainObjPtr
-virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
- int id,
- bool ref)
+
+virDomainObjPtr
+virDomainObjListFindByID(virDomainObjListPtr doms,
+ int id)
 {
 virDomainObjPtr obj;
+
 virObjectRWLockRead(doms);
 obj = virHashSearch(doms->objs, virDomainObjListSearchID, , NULL);
-if (ref) {
-virObjectRef(obj);
-virObjectRWUnlock(doms);
-}
+virObjectRef(obj);
+virObjectRWUnlock(doms);
 if (obj) {
 virObjectLock(obj);
 if (obj->removing) {
 virObjectUnlock(obj);
-if (ref)
-virObjectUnref(obj);
+virObjectUnref(obj);
 obj = NULL;
 }
 }
-if (!ref)
-virObjectRWUnlock(doms);
-return obj;
-}
-
-virDomainObjPtr
-virDomainObjListFindByID(virDomainObjListPtr doms,
- int id)
-{
-return virDomainObjListFindByIDInternal(doms, id, false);
-}
 
-virDomainObjPtr
-virDomainObjListFindByIDRef(virDomainObjListPtr doms,
-int id)
-{
-return virDomainObjListFindByIDInternal(doms, id, true);
+return obj;
 }
 
 
diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
index 1b77a95ba..7e2dece3a 100644
--- a/src/conf/virdomainobjlist.h
+++ b/src/conf/virdomainobjlist.h
@@ -34,8 +34,6 @@ virDomainObjListPtr virDomainObjListNew(void);
 
 virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms,
  int id);
-virDomainObjPtr virDomainObjListFindByIDRef(virDomainObjListPtr doms,
-int id);
 virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
const unsigned char *uuid);
 virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6f0cd9680..5b6a9d899 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -948,7 +948,6 @@ virDomainObjListCollect;
 virDomainObjListConvert;
 virDomainObjListExport;
 virDomainObjListFindByID;
-virDomainObjListFindByIDRef;
 virDomainObjListFindByName;
 virDomainObjListFindByUUID;
 virDomainObjListForEach;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index e76740247..6332b9ae2 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -443,7 +443,7 @@ libxlDomainShutdownThread(void *opaque)
 
 cfg = libxlDriverConfigGet(driver);
 
-vm = virDomainObjListFindByIDRef(driver->domains, ev->domid);
+vm = virDomainObjListFindByID(driver->domains, ev->domid);
 if (!vm) {
 VIR_INFO("Received event for unknown domain ID %d", ev->domid);
 goto cleanup;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 1379e9b83..5091592c6 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1090,7 +1090,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id)
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByIDRef(driver->domains, 

[libvirt] [PATCH 00/20] Rework/rename virDomainObjListFindBy{UUID|ID}Ref

[John Ferlan]

First, don't be scared off by 20 patches... They're not that difficult.

Second, don't be scared off by the all the various code touched.

Third, look at all the lines of code being removed! That's unlike
my normal patch series!

This is really the end game of what I started last year trying
to make object access code to be "more common". The last and worse
is/was the domain object code which really has a mish-mash of
usage models and is (to say the least) confusing depending on
the algorithm chosen (ByRef or not).

This series focuses on the various virDomainObjListFindBy* APIs
to do common things *and* allows the callers to use the common
virDomainObjEndAPI when done with the object rather than needing
to know whether they should virObjectUnlock and/or virObjectUnref.
There were one consumer (libxl) that leaked domain objs because
there was no Unref after a Ref on many API's. Additionally, there
was a possibility of a deadlock because of a missed Unlock.

This reduces a lot of code and reduces the number of virObjectUnlock
consumers.

Now *obviously* I don't have all these domain types available, but
since the goal was to remove the FindBy*Ref APIs, I had to touch
much more than I really want to. Of course only a couple are really
active anyway and I'm hoping they'll take a peek at their modules
(e.g. libxl, vz, and bhyve in particular).

Once/if this can get accomplished, fixing the Add and Remove API's
is the next task. Those are (to say the least) *really* confusing
because the caller should never have to call virObjectRef afterwards,
but some do because virDomainObjListAddLocked doesn't make enough
references on the obj (places the object in 2 lists, but only
accounts for 1). Lots of consumers that have to roll their own
"adjustments" because of that.

John Ferlan (20):
  bhyve: Use virDomainObjListFindBy{UUID|ID}Ref
  libxl: Fix possible deadlock in libxlDomainMigrateBegin3Params
  libxl: Properly cleanup after libxlDomObjFromDomain
  libxl: Use virDomainObjListFindBy{UUID|ID}Ref
  openvz: Cleanup indention
  openvz: Create accessors to virDomainObjListFindByUUID
  openvz: Add more descriptive error message on Find failure
  openvz:  Use virDomainObjListFindBy{UUID|ID}Ref
  uml: Create accessors to virDomainObjListFindByUUID
  uml: Add more specific error message on failed FindBy call
  uml: Use virDomainObjListFindBy{UUID|ID}Ref
  vmware: Create accessors to virDomainObjListFindByUUID
  vmware: Add more descriptive error message on Find failure
  vmware: Properly clean up in vmwareDomainLookupByName
  vmware: Use virDomainObjListFindBy{UUID|ID}Ref
  vz: Unify vzDomObjFromDomain{Ref}
  vz: Use virDomainObjListFindBy{UUID|ID}Ref
  test: Use virDomainObjListFindByUUIDRef
  conf: Rework/rename virDomainObjListFindByUUIDRef
  conf: Rework/rename virDomainObjListFindByIDRef

 src/bhyve/bhyve_driver.c |  52 ++
 src/conf/virdomainobjlist.c  |  66 ++--
 src/conf/virdomainobjlist.h  |   4 -
 src/libvirt_private.syms |   2 -
 src/libxl/libxl_domain.c |   2 +-
 src/libxl/libxl_driver.c |  99 ---
 src/libxl/libxl_migration.c  |   3 +-
 src/lxc/lxc_driver.c |   7 +-
 src/openvz/openvz_driver.c   | 392 ++-
 src/qemu/qemu_driver.c   |   9 +-
 src/test/test_driver.c   |   8 +-
 src/uml/uml_driver.c | 327 +++-
 src/util/virclosecallbacks.c |   4 +-
 src/vmware/vmware_driver.c   | 234 +-
 src/vz/vz_driver.c   | 117 +++--
 src/vz/vz_sdk.c  |  13 +-
 src/vz/vz_utils.c|  34 +---
 src/vz/vz_utils.h|   1 -
 18 files changed, 458 insertions(+), 916 deletions(-)

-- 
2.13.6

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

[libvirt] [PATCH 13/20] vmware: Add more descriptive error message on Find failure

[John Ferlan]

If vmwareDomainLookupByID or vmwareDomainLookupByName fails
to find a vm, let's be a bit more descriptive by providing
the failing id or name in the error message.

Signed-off-by: John Ferlan 
---
 src/vmware/vmware_driver.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index 121751e27..9cd0bc438 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -841,7 +841,8 @@ vmwareDomainLookupByID(virConnectPtr conn, int id)
 vmwareDriverUnlock(driver);
 
 if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching id '%d'"), id);
 goto cleanup;
 }
 
@@ -900,7 +901,8 @@ vmwareDomainLookupByName(virConnectPtr conn, const char 
*name)
 vmwareDriverUnlock(driver);
 
 if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching name '%s'"), name);
 goto cleanup;
 }
 
-- 
2.13.6

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

[libvirt] [PATCH 06/20] openvz: Create accessors to virDomainObjListFindByUUID

[John Ferlan]

Rather than repeat code throughout, create and use a couple of
accessors in order to lookup by UUID.

Signed-off-by: John Ferlan 
---
 src/openvz/openvz_driver.c | 266 +
 1 file changed, 76 insertions(+), 190 deletions(-)

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index a211c370e..167ba2f7a 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -87,6 +87,39 @@ static void openvzDriverUnlock(struct openvz_driver *driver)
 
 struct openvz_driver ovz_driver;
 
+
+static virDomainObjPtr
+openvzDomObjFromDomainLocked(struct openvz_driver *driver,
+ const unsigned char *uuid)
+{
+virDomainObjPtr vm;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
+virUUIDFormat(uuid, uuidstr);
+
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching uuid '%s'"), uuidstr);
+return NULL;
+}
+
+return vm;
+}
+
+
+static virDomainObjPtr
+openvzDomObjFromDomain(struct openvz_driver *driver,
+   const unsigned char *uuid)
+{
+virDomainObjPtr vm;
+
+openvzDriverLock(driver);
+vm = openvzDomObjFromDomainLocked(driver, uuid);
+openvzDriverUnlock(driver);
+return vm;
+}
+
+
 static int
 openvzDomainDefPostParse(virDomainDefPtr def,
  virCapsPtr caps ATTRIBUTE_UNUSED,
@@ -281,15 +314,8 @@ openvzDomainGetHostname(virDomainPtr dom, unsigned int 
flags)
 virDomainObjPtr vm;
 
 virCheckFlags(0, NULL);
-openvzDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-openvzDriverUnlock(driver);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, "%s",
-   _("no domain with matching uuid"));
-goto cleanup;
-}
+if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+return NULL;
 
 hostname = openvzVEGetStringParam(dom, "hostname");
 if (hostname == NULL)
@@ -359,18 +385,11 @@ static char *openvzDomainGetOSType(virDomainPtr dom)
 virDomainObjPtr vm;
 char *ret = NULL;
 
-openvzDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-openvzDriverUnlock(driver);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
-goto cleanup;
-}
+if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+return NULL;
 
 ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(vm->def->os.type)));
 
- cleanup:
 if (vm)
 virObjectUnlock(vm);
 return ret;
@@ -384,18 +403,11 @@ static virDomainPtr 
openvzDomainLookupByUUID(virConnectPtr conn,
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-openvzDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, uuid);
-openvzDriverUnlock(driver);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
-goto cleanup;
-}
+if (!(vm = openvzDomObjFromDomain(driver, uuid)))
+return NULL;
 
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
- cleanup:
 if (vm)
 virObjectUnlock(vm);
 return dom;
@@ -432,15 +444,8 @@ static int openvzDomainGetInfo(virDomainPtr dom,
 int state;
 int ret = -1;
 
-openvzDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-openvzDriverUnlock(driver);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, "%s",
-   _("no domain with matching uuid"));
-goto cleanup;
-}
+if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+return -1;
 
 if (openvzGetVEStatus(vm, , NULL) == -1)
 goto cleanup;
@@ -480,19 +485,11 @@ openvzDomainGetState(virDomainPtr dom,
 
 virCheckFlags(0, -1);
 
-openvzDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-openvzDriverUnlock(driver);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, "%s",
-   _("no domain with matching uuid"));
-goto cleanup;
-}
+if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
+return -1;
 
 ret = openvzGetVEStatus(vm, state, reason);
 
- cleanup:
 if (vm)
 virObjectUnlock(vm);
 return ret;
@@ -505,16 +502,11 @@ static int openvzDomainIsActive(virDomainPtr dom)
 virDomainObjPtr obj;
 int ret = -1;
 
-openvzDriverLock(driver);
-obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-openvzDriverUnlock(driver);
-if (!obj) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
-goto cleanup;
-}
+if (!(obj = openvzDomObjFromDomain(driver, dom->uuid)))
+return -1;
+
 ret = virDomainObjIsActive(obj);
 
- cleanup:
 if (obj)
 virObjectUnlock(obj);
 return ret;
@@ -527,16 +519,11 @@ static int 

[libvirt] [PATCH 09/20] uml: Create accessors to virDomainObjListFindByUUID

[John Ferlan]

Rather than repeat code throughout, create and use a couple of
accessors in order to lookup by UUID. This will also generate
a common error message including the failed uuidstr for lookup
rather than just returning nothing in some instances.

Signed-off-by: John Ferlan 
---
 src/uml/uml_driver.c | 244 +++
 1 file changed, 70 insertions(+), 174 deletions(-)

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index ab7fa7f27..ea8fca38b 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -163,6 +163,39 @@ umlVMDriverUnlock(void)
 umlDriverUnlock(uml_driver);
 }
 
+
+static virDomainObjPtr
+umlDomObjFromDomainLocked(struct uml_driver *driver,
+  const unsigned char *uuid)
+{
+virDomainObjPtr vm;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) {
+virUUIDFormat(uuid, uuidstr);
+
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching uuid '%s'"), uuidstr);
+return NULL;
+}
+
+return vm;
+}
+
+
+static virDomainObjPtr
+umlDomObjFromDomain(struct uml_driver *driver,
+const unsigned char *uuid)
+{
+virDomainObjPtr vm;
+
+umlDriverLock(driver);
+vm = umlDomObjFromDomainLocked(driver, uuid);
+umlDriverUnlock(driver);
+return vm;
+}
+
+
 static virNWFilterCallbackDriver umlCallbackDriver = {
 .name = "UML",
 .vmFilterRebuild = umlVMFilterRebuild,
@@ -1368,20 +1401,14 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr 
conn,
 }
 
 static virDomainPtr umlDomainLookupByUUID(virConnectPtr conn,
-const unsigned char *uuid)
+  const unsigned char *uuid)
 {
 struct uml_driver *driver = (struct uml_driver *)conn->privateData;
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-umlDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, uuid);
-umlDriverUnlock(driver);
-
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
-goto cleanup;
-}
+if (!(vm = umlDomObjFromDomain(driver, uuid)))
+return NULL;
 
 if (virDomainLookupByUUIDEnsureACL(conn, vm->def) < 0)
 goto cleanup;
@@ -1427,13 +1454,8 @@ static int umlDomainIsActive(virDomainPtr dom)
 virDomainObjPtr obj;
 int ret = -1;
 
-umlDriverLock(driver);
-obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-umlDriverUnlock(driver);
-if (!obj) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
-goto cleanup;
-}
+if (!(obj = umlDomObjFromDomain(driver, dom->uuid)))
+return -1;
 
 if (virDomainIsActiveEnsureACL(dom->conn, obj->def) < 0)
 goto cleanup;
@@ -1453,13 +1475,8 @@ static int umlDomainIsPersistent(virDomainPtr dom)
 virDomainObjPtr obj;
 int ret = -1;
 
-umlDriverLock(driver);
-obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-umlDriverUnlock(driver);
-if (!obj) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
-goto cleanup;
-}
+if (!(obj = umlDomObjFromDomain(driver, dom->uuid)))
+return -1;
 
 if (virDomainIsPersistentEnsureACL(dom->conn, obj->def) < 0)
 goto cleanup;
@@ -1478,13 +1495,8 @@ static int umlDomainIsUpdated(virDomainPtr dom)
 virDomainObjPtr obj;
 int ret = -1;
 
-umlDriverLock(driver);
-obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-umlDriverUnlock(driver);
-if (!obj) {
-virReportError(VIR_ERR_NO_DOMAIN, NULL);
-goto cleanup;
-}
+if (!(obj = umlDomObjFromDomain(driver, dom->uuid)))
+return -1;
 
 if (virDomainIsUpdatedEnsureACL(dom->conn, obj->def) < 0)
 goto cleanup;
@@ -1637,14 +1649,8 @@ static int umlDomainShutdownFlags(virDomainPtr dom,
 
 virCheckFlags(0, -1);
 
-umlDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-umlDriverUnlock(driver);
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN,
-   _("no domain with matching id %d"), dom->id);
-goto cleanup;
-}
+if (!(vm = umlDomObjFromDomain(driver, dom->uuid)))
+return -1;
 
 if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
 goto cleanup;
@@ -1683,12 +1689,8 @@ umlDomainDestroyFlags(virDomainPtr dom,
 virCheckFlags(0, -1);
 
 umlDriverLock(driver);
-vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-if (!vm) {
-virReportError(VIR_ERR_NO_DOMAIN,
-   _("no domain with matching id %d"), dom->id);
-goto cleanup;
-}
+if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid)))
+return -1;
 
 if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
@@ -1726,14 +1728,8 @@ static char 

[libvirt] [PATCH 05/20] openvz: Cleanup indention

[John Ferlan]

Some of the indents were only 2 spaces, make consistent w/ 4 spaces

Signed-off-by: John Ferlan 
---
 src/openvz/openvz_driver.c | 64 +++---
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index ebdc3890e..a211c370e 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -222,10 +222,10 @@ static int openvzSetInitialConfig(virDomainDefPtr vmdef)
 ret = 0;
 
  cleanup:
-  VIR_FREE(confdir);
-  virCommandFree(cmd);
+VIR_FREE(confdir);
+virCommandFree(cmd);
 
-  return ret;
+return ret;
 }
 
 
@@ -267,9 +267,9 @@ openvzSetDiskQuota(virDomainDefPtr vmdef,
 
 ret = 0;
  cleanup:
-  virCommandFree(cmd);
+virCommandFree(cmd);
 
-  return ret;
+return ret;
 }
 
 
@@ -633,40 +633,40 @@ static int openvzDomainSuspend(virDomainPtr dom)
 
 static int openvzDomainResume(virDomainPtr dom)
 {
-  struct openvz_driver *driver = dom->conn->privateData;
-  virDomainObjPtr vm;
-  const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, 
"--resume", NULL};
-  int ret = -1;
+struct openvz_driver *driver = dom->conn->privateData;
+virDomainObjPtr vm;
+const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, 
"--resume", NULL};
+int ret = -1;
 
-  openvzDriverLock(driver);
-  vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-  openvzDriverUnlock(driver);
+openvzDriverLock(driver);
+vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
+openvzDriverUnlock(driver);
 
-  if (!vm) {
-  virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
-  goto cleanup;
-  }
+if (!vm) {
+virReportError(VIR_ERR_NO_DOMAIN, "%s",
+   _("no domain with matching uuid"));
+goto cleanup;
+}
 
-  if (!virDomainObjIsActive(vm)) {
-  virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("Domain is not running"));
-  goto cleanup;
-  }
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Domain is not running"));
+goto cleanup;
+}
 
-  if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
-  openvzSetProgramSentinal(prog, vm->def->name);
-  if (virRun(prog, NULL) < 0)
-  goto cleanup;
-  virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, 
VIR_DOMAIN_RUNNING_UNPAUSED);
-  }
+if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
+openvzSetProgramSentinal(prog, vm->def->name);
+if (virRun(prog, NULL) < 0)
+goto cleanup;
+virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, 
VIR_DOMAIN_RUNNING_UNPAUSED);
+}
 
-  ret = 0;
+ret = 0;
 
  cleanup:
-  if (vm)
-  virObjectUnlock(vm);
-  return ret;
+if (vm)
+virObjectUnlock(vm);
+return ret;
 }
 
 static int
-- 
2.13.6

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

[libvirt] [PATCH 04/20] libxl: Use virDomainObjListFindBy{UUID|ID}Ref

[John Ferlan]

For libxlDomainLookupByID and libxlDomainLookupByUUID let's
return a locked and referenced @vm object so that callers can
then use the common and more consistent virDomainObjEndAPI in
order to handle cleanup rather than needing to know that the
returned object is locked and calling virObjectUnlock.

The LookupByName already returns the ref counted and locked object,
so this will make things more consistent.

Signed-off-by: John Ferlan 
---
 src/libxl/libxl_driver.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9aa4a293c..e78fe2d4b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1090,7 +1090,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id)
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByID(driver->domains, id);
+vm = virDomainObjListFindByIDRef(driver->domains, id);
 if (!vm) {
 virReportError(VIR_ERR_NO_DOMAIN, NULL);
 goto cleanup;
@@ -1102,8 +1102,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id)
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
@@ -1114,7 +1113,7 @@ libxlDomainLookupByUUID(virConnectPtr conn, const 
unsigned char *uuid)
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByUUID(driver->domains, uuid);
+vm = virDomainObjListFindByUUIDRef(driver->domains, uuid);
 if (!vm) {
 virReportError(VIR_ERR_NO_DOMAIN, NULL);
 goto cleanup;
@@ -1126,8 +1125,7 @@ libxlDomainLookupByUUID(virConnectPtr conn, const 
unsigned char *uuid)
 dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return dom;
 }
 
-- 
2.13.6

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

[libvirt] [PATCH 03/20] libxl: Properly cleanup after libxlDomObjFromDomain

[John Ferlan]

Commit id '9ac945078' altered libxlDomObjFromDomain to return
a locked *and* ref counted object for some specific purposes;
however, it neglected to alter all the consumers of the helper
to use virDomainObjEndAPI thus leaving many objects with extra
ref counts.

The two consumers for libxlDomainMigrationConfirm would also
originally use the libxlDomObjFromDomain API (either from
libxlDomainMigrateConfirm3Params or libxlDoMigrateP2P via
libxlDomainMigrationPerformP2P and libxlDomainMigratePerform3Params.

Signed-off-by: John Ferlan 
---
 src/libxl/libxl_driver.c| 86 -
 src/libxl/libxl_migration.c |  3 +-
 2 files changed, 31 insertions(+), 58 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b5101626e..9aa4a293c 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1322,8 +1322,7 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int 
flags)
 }
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 virObjectUnref(cfg);
 return ret;
 }
@@ -1373,8 +1372,7 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
 }
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 virObjectUnref(cfg);
 return ret;
 }
@@ -1459,8 +1457,7 @@ libxlDomainGetOSType(virDomainPtr dom)
 goto cleanup;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return type;
 }
 
@@ -1479,8 +1476,7 @@ libxlDomainGetMaxMemory(virDomainPtr dom)
 ret = virDomainDefGetMemoryTotal(vm->def);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -1658,8 +1654,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr 
info)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 virObjectUnref(cfg);
 return ret;
 }
@@ -1685,8 +1680,7 @@ libxlDomainGetState(virDomainPtr dom,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -2110,8 +2104,7 @@ libxlDomainHasManagedSaveImage(virDomainPtr dom, unsigned 
int flags)
 ret = vm->hasManagedSave;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -2140,8 +2133,7 @@ libxlDomainManagedSaveRemove(virDomainPtr dom, unsigned 
int flags)
 
  cleanup:
 VIR_FREE(name);
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -2352,8 +2344,7 @@ libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int 
flags)
 ret = virDomainDefGetVcpus(def);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -2484,8 +2475,7 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
libxl_get_max_cpus(cfg->ctx), NULL);
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 virObjectUnref(cfg);
 return ret;
 }
@@ -2548,8 +2538,7 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr 
info, int maxinfo,
 ret = maxinfo;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 virObjectUnref(cfg);
 return ret;
 }
@@ -2580,8 +2569,7 @@ libxlDomainGetXMLDesc(virDomainPtr dom, unsigned int 
flags)
  virDomainDefFormatConvertXMLFlags(flags));
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 virObjectUnref(cfg);
 return ret;
 }
@@ -2901,8 +2889,7 @@ libxlDomainUndefineFlags(virDomainPtr dom,
 
  cleanup:
 VIR_FREE(name);
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 if (event)
 libxlDomainEventQueue(driver, event);
 virObjectUnref(cfg);
@@ -4261,8 +4248,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char 
*xml,
  cleanup:
 virDomainDefFree(vmdef);
 virDomainDeviceDefFree(dev);
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 virObjectUnref(cfg);
 return ret;
 }
@@ -4394,8 +4380,7 @@ libxlDomainGetAutostart(virDomainPtr dom, int *autostart)
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 return ret;
 }
 
@@ -4521,8 +4506,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int 
*nparams)
 ignore_value(VIR_STRDUP(ret, name));
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 virObjectUnref(cfg);
 return ret;
 }
@@ -4587,8 +4571,7 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
 ret = 0;
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virDomainObjEndAPI();
 virObjectUnref(cfg);
 return ret;
 }
@@ -4750,8 +4733,7 @@ libxlDomainOpenConsole(virDomainPtr dom,
 }
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+

Re: [libvirt] [PATCH 1/8] tests: Rename pseries-features-hpt test

[Andrea Bolognani]

On Fri, 2018-03-09 at 16:07 +0100, Andrea Bolognani wrote:
> We're going to use the same test case to exercise all optional
> pSeries features, so a more generic name is needed.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .../{pseries-features-hpt.args => pseries-features.args}  | 0
>  .../{pseries-features-hpt.xml => pseries-features.xml}| 0
>  tests/qemuxml2argvtest.c  | 4 
> ++--
>  tests/qemuxml2xmloutdata/pseries-features-hpt.xml | 1 -
>  tests/qemuxml2xmloutdata/pseries-features.xml | 1 +
>  tests/qemuxml2xmltest.c   | 2 +-
>  6 files changed, 4 insertions(+), 4 deletions(-)
>  rename tests/qemuxml2argvdata/{pseries-features-hpt.args => 
> pseries-features.args} (100%)
>  rename tests/qemuxml2argvdata/{pseries-features-hpt.xml => 
> pseries-features.xml} (100%)
>  delete mode 12 tests/qemuxml2xmloutdata/pseries-features-hpt.xml
>  create mode 12 tests/qemuxml2xmloutdata/pseries-features.xml

This does no longer apply cleanly now that 07160b65db94 has been
pushed, but the conflict is trivial.

At any rate, I've updated the GitHub branch so that it's based on
b9b9195f15c9 instead of 90d7262552e8, which means anyone fetching
from there won't have to deal with the conflict at all.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v7 00/11] qemu: Validate PCI controller options

[Laine Stump]

On 03/08/2018 08:34 AM, Andrea Bolognani wrote:
> Applies cleanly on top of b932ed69f6664f42e211bdde84c8ab04e1f19033.
>
> Patches 2/11 and 4/11 are the only ones missing R-bs, everything
> else hasn't been significantly altered since [v5].

This all looks good now. I replied to 2 & 4 with R-b's. Sorry I didn't
see this yesterday (was busy at the doctor all morning), so it's all
ACKed and pushable!

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

Re: [libvirt] [PATCH v7 04/11] qemu: Validate PCI controller options (targetIndex)

[Laine Stump]

On 03/08/2018 08:34 AM, Andrea Bolognani wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1483816
>
> Signed-off-by: Andrea Bolognani 

Reviewed-by: Laine Stump 

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

Re: [libvirt] [PATCH v7 02/11] qemu: Validate PCI controller options (modelName)

[Laine Stump]

On 03/08/2018 08:34 AM, Andrea Bolognani wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1483816
>
> Signed-off-by: Andrea Bolognani 
> ---
>  src/qemu/qemu_domain.c | 226 
> +++--
>  1 file changed, 142 insertions(+), 84 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f1139cbac3..d8669ee9b3 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4274,7 +4274,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const 
> virDomainControllerDef *contro
>  {
>  virDomainControllerModelPCI model = controller->model;
>  const virDomainPCIControllerOpts *pciopts;
> [...]
>  
>  
> +#define virReportControllerMissingOption(cont, model, modelName, option) \
> +virReportError(VIR_ERR_INTERNAL_ERROR, \
> +   _("Required option '%s' is not set for PCI controller " \
> + "with index '%d', model '%s' and modelName '%s'"), \
> +   (option), (cont->idx), (model), (modelName));
> +#define virReportControllerInvalidOption(cont, model, modelName, option) \
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
> +   _("Option '%s' is not valid for PCI controller " \
> + "with index '%d', model '%s' and modelName '%s'"), \
> +   (option), (cont->idx), (model), (modelName));
> +#define virReportControllerInvalidValue(cont, model, modelName, option) \
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
> +   _("Option '%s' has invalid value for PCI controller " \
> + "with index '%d', model '%s' and modelName '%s'"), \
> +   (option), (cont->idx), (model), (modelName));
> +
> +
>  static int
>  qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
>   const virDomainDef *def,
> @@ -4566,10 +4499,135 @@ qemuDomainDeviceDefValidateControllerPCI(const 
> virDomainControllerDef *cont,
>  return -1;
>  }
>  
> +/* modelName */
> +switch ((virDomainControllerModelPCI) cont->model) {
> +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
> +case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
> +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
> +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
> +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
> +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
> +case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
> +/* modelName should have been set automatically */
> +if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) 
> {
> +virReportControllerMissingOption(cont, model, modelName, 
> "modelName");

"Required option 'modelName' is not set for PCI controller with index
'1', model 'pcie-root-port', modelName 'none'."

Yep, looks good to me.

> +return -1;
> +}
> +break;
> +
> +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
> +/* modelName must be set for pSeries guests, but it's an error
> + * for it to be set for any other guest */
> +if (qemuDomainIsPSeries(def)) {
> +if (pciopts->modelName == 
> VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
> +virReportControllerMissingOption(cont, model, modelName, 
> "modelName");
> +return -1;
> +}
> +} else {
> +if (pciopts->modelName != 
> VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
> +virReportControllerInvalidOption(cont, model, modelName, 
> "modelName");
> +return -1;

...and since we've already errored out on modelName values outside the
accepted range, this is okay too.

> [...]

Reviewed-by: Laine Stump 


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

[libvirt] [PATCH 2/8] tests: Add capabilities data for QEMU 2.12

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 .../caps_2.12.0-gicv2.aarch64.replies  | 17145 +++
 .../caps_2.12.0-gicv2.aarch64.xml  |   318 +
 .../caps_2.12.0-gicv3.aarch64.replies  | 17145 +++
 .../caps_2.12.0-gicv3.aarch64.xml  |   318 +
 .../qemucapabilitiesdata/caps_2.12.0.ppc64.replies | 21091 +++
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |  1093 +
 .../caps_2.12.0.x86_64.replies | 19113 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |  1270 ++
 tests/qemucapabilitiestest.c   | 4 +
 9 files changed, 77497 insertions(+)
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml

diff --git a/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.replies
new file mode 100644
index 00..9b1ed887e9
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.replies
@@ -0,0 +1,17145 @@
+{
+  "QMP": {
+"version": {
+  "qemu": {
+"micro": 50,
+"minor": 11,
+"major": 2
+  },
+  "package": " (v2.11.0-2110-gf6d81cd)"
+},
+"capabilities": [
+]
+  }
+}
[...]
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
new file mode 100644
index 00..c81dfcba43
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
@@ -0,0 +1,318 @@
+
+  0
+  0
+  0
+  
[...]
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.replies
new file mode 100644
index 00..9a12b14569
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.replies
@@ -0,0 +1,17145 @@
+{
+  "QMP": {
+"version": {
+  "qemu": {
+"micro": 50,
+"minor": 11,
+"major": 2
+  },
+  "package": " (v2.11.0-2110-gf6d81cd)"
+},
+"capabilities": [
+]
+  }
+}
[...]
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml
new file mode 100644
index 00..fae7c26538
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml
@@ -0,0 +1,318 @@
+
+  0
+  0
+  0
+  
[...]
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
new file mode 100644
index 00..0ccf0ad403
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
@@ -0,0 +1,21091 @@
+{
+  "QMP": {
+"version": {
+  "qemu": {
+"micro": 50,
+"minor": 11,
+"major": 2
+  },
+  "package": " (v2.11.0-2110-gf6d81cd)"
+},
+"capabilities": [
+]
+  }
+}
[...]
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
new file mode 100644
index 00..e82a7cd5dd
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -0,0 +1,1093 @@
+
+  0
+  0
+  0
+  
[...]
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
new file mode 100644
index 00..c161dd006b
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
@@ -0,0 +1,19113 @@
+{
+  "QMP": {
+"version": {
+  "qemu": {
+"micro": 50,
+"minor": 11,
+"major": 2
+  },
+  "package": " (v2.11.0-2110-gf6d81cdec8-dirty)"
+},
+"capabilities": [
+]
+  }
+}
[...]
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
new file mode 100644
index 00..50b2ac52d3
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -0,0 +1,1270 @@
+
+  0
+  0
+  0
+  
[...]
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index f9e8e187c2..dcad02588c 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -177,13 +177,17 @@ mymain(void)
 DO_TEST("x86_64", "caps_2.8.0");
 DO_TEST("x86_64", "caps_2.9.0");
 DO_TEST("x86_64", "caps_2.10.0");
+DO_TEST("x86_64", "caps_2.12.0");
 DO_TEST("aarch64", "caps_2.6.0-gicv2");
 DO_TEST("aarch64", "caps_2.6.0-gicv3");
 DO_TEST("aarch64", "caps_2.10.0-gicv2");
 

Re: [libvirt] [Qemu-devel] [RFC] Defining firmware (OVMF, et al) metadata format & file

[Laszlo Ersek]

On 03/09/18 15:27, Gerd Hoffmann wrote:
>   Hi,
> 
>> For OVMF (x86), I guess the initial set of properties should come from
>> the "-D FOO[=BAR]" build flags that OVMF currently supports. (The list
>> might grow or change incompatibly over time, so this is just a raw
>> starter idea.)
> 
>> (0) ARCH (one of IA32, IA32X64, X64) -- the bitnesses of the PEI and DXE
>> phases of the firmware.
>>
>> IA32 stands for "32-bit PEI and DXE". Such firmware is usable for
>> booting 32-bit OSes only, and runs on both qemu-system-i386 and
>> qemu-system-x86_64.
>>
>> IA32X64 stands for "32-bit PEI, 64-bit DXE". Needs qemu-system-x86_64
>> and runs 64-bit OSes only.
>>
>> X64 stands for "64-bit PEI and DXE". Needs qemu-system-x86_64 and runs
>> 64-bit OSes only.
> 
> I'd simply put that info the 'arch' bucket.  IA32 gets 'i386' (assuming
> we simply use qemu target names), IA32X64 + X64 get 'x86_64'.
> 
>> (1) SECURE_BOOT_ENABLE (boolean) -- whether the Secure Boot UEFI feature
>> is built into the firmware image.
> 
> Probably useful for upper management (above libvirt), for enabling
> secure boot key enrollment etc. in the UI.
> 
>> (2) SMM_REQUIRE (boolean) -- whether the SMM driver stack is included in
>> the firmware.
> 
> Needed (libvirt must enable smm).
> 
>> (3) HTTP_BOOT_ENABLE (boolean) -- whether UEFI HTTP boot is available in
>> the firmware image, in addition to the default PXE boot.
> 
>> (4) TLS_ENABLE (boolean) -- configurable independently of
>> HTTP_BOOT_ENABLE, but only really makes sense in combination.
> 
>> (5) NETWORK_IP6_ENABLE (boolean) -- determines whether IPv6 support is
>> available.
> 
> Do we actually need formal flags for these?  Which application would
> use them?
> 
> For humans this can go into the freeform "description" text field.
> 
>> (6) FD_SIZE_IN_KB (one of: 1024, 2048, 4096) -- the size of the combined
>> firmware image (executable portion and variable store together), in KB.
> 
> Hmm, dunno.  Might be useful as varstores are not portable between
> builds of different sizes, so libvirt could use this for sanity checks.
> 
>> (7) DEBUG_ON_SERIAL_PORT (boolean) -- whether OVMF sends its debug
>> messages to the QEMU debug IO port, or to the serial port.
> 
>> (8) SOURCE_DEBUG_ENABLE (boolean) -- whether OVMF includes the edk2
>> debug agent that allows it to be debugged from a proprietary debugger
>> program, likely connected via the emulated serial port.
> 
>> (9) CSM_ENABLE (boolean) -- whether OVMF includes traditional BIOS
>> support, by including the SeaBIOS Compatibility Support Module.
> 
> Same question as for 3+4+5: Do we really need those?
> 
>> Given that the SeaBIOS CSM itself can be built with various
>> configurations, it might make sense to list further properties when this
>> property is enabled.
> 
> I doubt this is useful, established practice is that people just use
> seabios directly instead if using the ovmf+csm combo.
> 
>> (10) E1000_ENABLE (boolean) -- whether OVMF includes the
>> non-redistributable, binary only E1000(E) UEFI driver module from Intel
>> (previously known as "PROEFI", more recently known as "BootUtil").
> 
>> (11) USE_OLD_SHELL (boolean) -- whether the UEFI shell implementation
>> built into OVMF is the old (EDK1 / EFI-1 style) shell which lives in a
>> separate repository, or the new (EDK2 / UEFI-2 style) shell which lives
>> within the edk2 project.
> 
>> (12) TOOLCHAIN (string): the edk2 toolchain identifier with which the
>> firmware was built.
> 
>> (13) TARGET (one of NOOPT, DEBUG, RELEASE) -- the target for which the
>> firmware image was built.
> 
> Same question as for 3+4+5+7+8+9: Do we really need those?

I can't judge that. To me all these flags are meaningful. It doesn't
hurt to keep them spelled out in the metadata; they don't take much
space, they are easy to ignore, and if someone searches for them, they
are there.

I'll offer an analogy. Sometimes it would be useful to know the full
kernel config used on a distro's Live or Installer ISO. Just today I got
a question off-list as to whether the aarch64 Fedora 28 Live iso has
CONFIG_ARM64_PMEM enabled or not. I have no clue, but this type of
question is not far from the domain of "libosinfo" (which basically
tells you what OS release has drivers for what devices). Is "libfwinfo"
such a stretch? Maybe. :)

So, whether this level of detail is useful to end-users and/or
management applications, I cannot tell; I just wanted to expose the
"current spectrum" (for OVMF and ArmVirtQemu) in reasonable detail.

Thanks
Laszlo

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

Re: [libvirt] [Qemu-devel] [RFC] Defining firmware (OVMF, et al) metadata format & file

[Laszlo Ersek]

On 03/09/18 12:27, Kashyap Chamarthy wrote:
> On Thu, Mar 08, 2018 at 09:47:27PM +0100, Laszlo Ersek wrote:
>> On 03/08/18 16:47, Daniel P. Berrangé wrote:
>>> On Thu, Mar 08, 2018 at 12:10:30PM +0100, Laszlo Ersek wrote:
> 
> [...]
> 
 For OVMF (x86), I guess the initial set of properties should come from
 the "-D FOO[=BAR]" build flags that OVMF currently supports. (The list
 might grow or change incompatibly over time, so this is just a raw
 starter idea.)
>>>
>>> I really don't want to see us using firmware implementation specific
>>> property names in these files. It means libvirt will require knowledge
>>> of what each different firmware's property names mean.
>>>
>>> We need to have some core standardized set of property names that can
>>> be provided by any firmware implementation using the same terminology.
>>>
>>> If we want to /also/ provide some extra firmeware-specific property
>>> names that would be ok for informative purposes, but when lbivirt is
>>> picking which firmware file to use, it would only ever look at the
>>> standardized property names/values.
>>
>> This is a reasonable requirement from the libvirt side.
>>
>> Unfortunately (or not), it requires someone (or a tight group of people)
>> to collect the features of all virtual firmwares in existence, and
>> extract a common set of properties that maps back to each firmware one
>> way or another. 
> 
> Hmm, if people consider the above worthwhile (no clue how much time &
> investigation it takes to arrive at a common set of properties) maybe
> slowly we should start collecting such a page?  From a quick look up,
> list of open source firmware implementations I found so (besides OVMF &
> ArmVirt):
> 
>   - OpenBIOS
>   - SmartFirmware
>   - OpenBoot
>   - CoreBoot
>   - U-Boot
>   - SLOF
>   - ...
> 
> Source: https://en.wikipedia.org/wiki/OpenBIOS
> 
> I notice you said "virtual firmwares".  I couldn't find such a list from
> my look up.
> 
> Hmm, I also wonder if the "arriving at a common set of properties across
> existing virtual firmwares" is an absolute blocker.

That's for Daniel to decide. I can't sensibly generalize from OVMF &
ArmVirt to other firmwares, without knowing them.

Thanks
Laszlo

> 
>> This is not unusual (basically this is how all standards
>> bodies work that intend to codify existing practice), it just needs a
>> bunch of work and coordination. We'll have to maintain a registry.
>>
>> Personally I can't comment on anything else than OVMF and the ArmVirt
>> firmwares.
>>
>> Thanks,
>> Laszlo
> 

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

[libvirt] [PATCH 8/8] news: Update for the HTM pSeries feature

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 docs/news.xml | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index a51ca973e4..fc56f776ae 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -35,6 +35,15 @@
 
   
 
+  
+
+  qemu: Implement the HTM pSeries features
+
+
+  Users can now decide whether HTM (Hardware Transactional Memory)
+  support should be available to the guest.
+
+  
 
 
   
-- 
2.14.3

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

[libvirt] [PATCH 0/8] Add optional pSeries features (HTM)

[Andrea Bolognani]

Applies cleanly on top of 90d7262552e8de3b067239b215903144a285723d.

Patch 2/8 is fairly big because of all the capabilities data being
added: I'm sending a redacted version to the list, the rest of the
code can be grabbed from

  https://github.com/andreabolognani/libvirt/tree/pseries-caps

Changes from [RFC v3]:

  * can be considered for merging, since the QEMU part already has;
  * fix compatibility issues with QEMU <2.12 spotted by Shivaprasad;
  * drop all features except for HTM, at least for the time being;
  * add documentation.

Changes from [RFC v2]:

  * use qom-list-properties to probe availability;
  * test all features with a single XML file.

Changes from [RFC v1]:

  * don't nest features inside a  element;
  * implement all optional features.

[RFC v3] https://www.redhat.com/archives/libvir-list/2018-March/msg00042.html
[RFC v2] https://www.redhat.com/archives/libvir-list/2018-February/msg00310.html
[RFC v1] https://www.redhat.com/archives/libvir-list/2018-January/msg00779.html

Andrea Bolognani (8):
  tests: Rename pseries-features-hpt test
  tests: Add capabilities data for QEMU 2.12
  qemu: Add capability for qom-list-properties
  qemu: Rename virQEMUCapsObjectProps* -> virQEMUCapsDeviceProps*
  qemu: Extract generic part from qemuMonitorJSONGetDeviceProps()
  qemu: Add capability for the HTM pSeries feature
  qemu: Implement the HTM pSeries feature
  news: Update for the HTM pSeries feature

 docs/formatdomain.html.in  | 7 +
 docs/news.xml  | 9 +
 docs/schemas/domaincommon.rng  | 5 +
 src/conf/domain_conf.c |19 +
 src/conf/domain_conf.h | 1 +
 src/qemu/qemu_capabilities.c   |   281 +-
 src/qemu/qemu_capabilities.h   | 2 +
 src/qemu/qemu_command.c|   149 +
 src/qemu/qemu_domain.c |13 +
 src/qemu/qemu_monitor.c|13 +
 src/qemu/qemu_monitor.h| 3 +
 src/qemu/qemu_monitor_json.c   |40 +-
 src/qemu/qemu_monitor_json.h   | 5 +
 .../caps_2.12.0-gicv2.aarch64.replies  | 17153 +++
 .../caps_2.12.0-gicv2.aarch64.xml  |   319 +
 .../caps_2.12.0-gicv3.aarch64.replies  | 17153 +++
 .../caps_2.12.0-gicv3.aarch64.xml  |   319 +
 .../qemucapabilitiesdata/caps_2.12.0.ppc64.replies | 21247 +++
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |  1095 +
 .../caps_2.12.0.x86_64.replies | 19121 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |  1271 ++
 tests/qemucapabilitiestest.c   | 4 +
 .../pseries-features-invalid-machine.xml   | 1 +
 ...ies-features-hpt.args => pseries-features.args} | 2 +-
 ...eries-features-hpt.xml => pseries-features.xml} | 1 +
 tests/qemuxml2argvtest.c   | 5 +-
 tests/qemuxml2xmloutdata/pseries-features-hpt.xml  | 1 -
 tests/qemuxml2xmloutdata/pseries-features.xml  | 1 +
 tests/qemuxml2xmltest.c| 3 +-
 29 files changed, 78113 insertions(+), 130 deletions(-)
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
 rename tests/qemuxml2argvdata/{pseries-features-hpt.args => 
pseries-features.args} (87%)
 rename tests/qemuxml2argvdata/{pseries-features-hpt.xml => 
pseries-features.xml} (97%)
 delete mode 12 tests/qemuxml2xmloutdata/pseries-features-hpt.xml
 create mode 12 tests/qemuxml2xmloutdata/pseries-features.xml

-- 
2.14.3

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

[libvirt] [PATCH 3/8] qemu: Add capability for qom-list-properties

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_capabilities.c | 4 +++-
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml| 1 +
 6 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3eb5ed6d1a..c70bd27f18 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "pl011",
   "machine.pseries.max-cpu-compat",
   "dump-completed",
+  "qom-list-properties",
 );
 
 
@@ -1579,7 +1580,8 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
 { "query-qmp-schema", QEMU_CAPS_QUERY_QMP_SCHEMA },
 { "query-cpu-model-expansion", QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION},
 { "query-cpu-definitions", QEMU_CAPS_QUERY_CPU_DEFINITIONS},
-{ "query-named-block-nodes", QEMU_CAPS_QUERY_NAMED_BLOCK_NODES}
+{ "query-named-block-nodes", QEMU_CAPS_QUERY_NAMED_BLOCK_NODES },
+{ "qom-list-properties", QEMU_CAPS_QOM_LIST_PROPERTIES },
 };
 
 struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c2ec2be193..ce07dfd6b1 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -444,6 +444,7 @@ typedef enum {
 QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */
 QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine 
pseries,max-cpu-compat= */
 QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */
+QEMU_CAPS_QOM_LIST_PROPERTIES, /* qom-list-properties QMP command */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
index c81dfcba43..9c4e40973b 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
@@ -187,6 +187,7 @@
   
   
   
+  
   2011050
   0
   315524
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml
index fae7c26538..06e0068cbd 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0-gicv3.aarch64.xml
@@ -187,6 +187,7 @@
   
   
   
+  
   2011050
   0
   315524
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index e82a7cd5dd..a0df689c25 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -185,6 +185,7 @@
   
   
   
+  
   2011050
   0
   393199
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
index 50b2ac52d3..fbdde1733f 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -228,6 +228,7 @@
   
   
   
+  
   2011050
   0
   364740
-- 
2.14.3

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

[libvirt] [PATCH 6/8] qemu: Add capability for the HTM pSeries feature

[Andrea Bolognani]

This is the first capability that depends on an object property,
so we need to introduce a couple new functions at the same time.

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_capabilities.c   |  38 +
 src/qemu/qemu_capabilities.h   |   1 +
 src/qemu/qemu_monitor.c|  13 ++
 src/qemu/qemu_monitor.h|   3 +
 src/qemu/qemu_monitor_json.c   |  10 ++
 src/qemu/qemu_monitor_json.h   |   5 +
 .../caps_2.12.0-gicv2.aarch64.replies  |  24 ++-
 .../caps_2.12.0-gicv2.aarch64.xml  |   2 +-
 .../caps_2.12.0-gicv3.aarch64.replies  |  24 ++-
 .../caps_2.12.0-gicv3.aarch64.xml  |   2 +-
 .../qemucapabilitiesdata/caps_2.12.0.ppc64.replies | 170 -
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |   3 +-
 .../caps_2.12.0.x86_64.replies |  30 ++--
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   2 +-
 14 files changed, 289 insertions(+), 38 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 83ec8a67d5..0165de0407 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -460,6 +460,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "machine.pseries.max-cpu-compat",
   "dump-completed",
   "qom-list-properties",
+  "machine.pseries.cap-htm",
 );
 
 
@@ -1950,6 +1951,21 @@ static struct virQEMUCapsObjectTypeProps 
virQEMUCapsDeviceProps[] = {
   QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE },
 };
 
+/* Object properties.
+ *
+ * The following can be probed using the qom-list-properties QMP command
+ */
+
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsSPAPRMachine[] = {
+{ "cap-htm", QEMU_CAPS_MACHINE_PSERIES_CAP_HTM },
+};
+
+static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = {
+{ "spapr-machine", virQEMUCapsObjectPropsSPAPRMachine,
+  ARRAY_CARDINALITY(virQEMUCapsObjectPropsSPAPRMachine),
+  -1 },
+};
+
 /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format */
 static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/options/+gluster/debug-level", 
QEMU_CAPS_GLUSTER_DEBUG_LEVEL},
@@ -2876,6 +2892,28 @@ virQEMUCapsProbeQMPObjects(virQEMUCapsPtr qemuCaps,
 virStringListFreeCount(values, nvalues);
 }
 
+/* If the qom-list-properties QMP command is available, then we
+ * can probe objects in addition to devices */
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QOM_LIST_PROPERTIES)) {
+for (i = 0; i < ARRAY_CARDINALITY(virQEMUCapsObjectProps); i++) {
+const char *type = virQEMUCapsObjectProps[i].type;
+int cap = virQEMUCapsObjectProps[i].capsCondition;
+
+if (cap >= 0 && !virQEMUCapsGet(qemuCaps, cap))
+continue;
+
+if ((nvalues = qemuMonitorGetObjectProps(mon,
+ type,
+ )) < 0)
+return -1;
+virQEMUCapsProcessStringFlags(qemuCaps,
+  virQEMUCapsObjectProps[i].nprops,
+  virQEMUCapsObjectProps[i].props,
+  nvalues, values);
+virStringListFreeCount(values, nvalues);
+}
+}
+
 /* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_SPICEVMC))
 virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index ce07dfd6b1..62a1138130 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -445,6 +445,7 @@ typedef enum {
 QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine 
pseries,max-cpu-compat= */
 QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */
 QEMU_CAPS_QOM_LIST_PROPERTIES, /* qom-list-properties QMP command */
+QEMU_CAPS_MACHINE_PSERIES_CAP_HTM, /* -machine pseries,cap-htm= */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 8ec759fe3f..6e43d268e1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3954,6 +3954,19 @@ qemuMonitorGetDeviceProps(qemuMonitorPtr mon,
 }
 
 
+int
+qemuMonitorGetObjectProps(qemuMonitorPtr mon,
+  const char *type,
+  char ***props)
+{
+VIR_DEBUG("type=%s props=%p", type, props);
+
+QEMU_CHECK_MONITOR_JSON(mon);
+
+return qemuMonitorJSONGetObjectProps(mon, type, props);
+}
+
+
 char *
 qemuMonitorGetTargetArch(qemuMonitorPtr mon)
 {
diff --git a/src/qemu/qemu_monitor.h 

[libvirt] [PATCH 7/8] qemu: Implement the HTM pSeries feature

[Andrea Bolognani]

This is the first in a list of pSeries-specific optional features
that have recently been introduced in QEMU. Along with the feature
proper, some generic code that will make it easier to implement
subsequent features is introduced as well.

Signed-off-by: Andrea Bolognani 
---
 docs/formatdomain.html.in  |   7 +
 docs/schemas/domaincommon.rng  |   5 +
 src/conf/domain_conf.c |  19 +++
 src/conf/domain_conf.h |   1 +
 src/qemu/qemu_command.c| 149 +
 src/qemu/qemu_domain.c |  13 ++
 .../pseries-features-invalid-machine.xml   |   1 +
 tests/qemuxml2argvdata/pseries-features.args   |   2 +-
 tests/qemuxml2argvdata/pseries-features.xml|   1 +
 tests/qemuxml2argvtest.c   |   1 +
 tests/qemuxml2xmltest.c|   1 +
 11 files changed, 199 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6fd2189cd2..51400afa49 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2057,6 +2057,13 @@
   the attribute is not defined, the hypervisor default will be used.
   Since 3.10.0 (QEMU/KVM only)
   
+  htm
+  Configure HTM (Hardware Transational Memory) availability for
+  pSeries guests. Possible values for the state attribute
+  are on and off. If the attribute is not
+  defined, the hypervisor default will be used.
+  Since 4.2.0 (QEMU/KVM only)
+  
   vmcoreinfo
   Enable QEMU vmcoreinfo device to let the guest kernel save debug
   details. Since 3.10.0 (QEMU only)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 8165e699d6..b4143f5bc3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4797,6 +4797,11 @@
   
 
   
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 70b19311b4..98897d3a63 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -152,6 +152,7 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
   "ioapic",
   "hpt",
   "vmcoreinfo",
+  "htm",
 );
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
@@ -19297,6 +19298,22 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_HTM:
+if (!(tmp = virXMLPropString(nodes[i], "state"))) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("missing state attribute '%s' of feature 
'%s'"),
+   tmp, virDomainFeatureTypeToString(val));
+goto error;
+}
+if ((def->features[val] = virTristateSwitchTypeFromString(tmp)) < 
0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown state attribute '%s' of feature 
'%s'"),
+   tmp, virDomainFeatureTypeToString(val));
+goto error;
+}
+VIR_FREE(tmp);
+break;
+
 /* coverity[dead_error_begin] */
 case VIR_DOMAIN_FEATURE_LAST:
 break;
@@ -21384,6 +21401,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 case VIR_DOMAIN_FEATURE_VMPORT:
 case VIR_DOMAIN_FEATURE_SMM:
 case VIR_DOMAIN_FEATURE_VMCOREINFO:
+case VIR_DOMAIN_FEATURE_HTM:
 if (src->features[i] != dst->features[i]) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("State of feature '%s' differs: "
@@ -26845,6 +26863,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
 case VIR_DOMAIN_FEATURE_VMPORT:
 case VIR_DOMAIN_FEATURE_SMM:
+case VIR_DOMAIN_FEATURE_HTM:
 switch ((virTristateSwitch) def->features[i]) {
 case VIR_TRISTATE_SWITCH_LAST:
 case VIR_TRISTATE_SWITCH_ABSENT:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5859c8f4b1..b87a9d9de2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1746,6 +1746,7 @@ typedef enum {
 VIR_DOMAIN_FEATURE_IOAPIC,
 VIR_DOMAIN_FEATURE_HPT,
 VIR_DOMAIN_FEATURE_VMCOREINFO,
+VIR_DOMAIN_FEATURE_HTM,
 
 VIR_DOMAIN_FEATURE_LAST
 } virDomainFeature;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fa0aa5d5c3..d58211c4c6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7117,6 +7117,113 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
 return 0;
 }
 
+static int

[libvirt] [PATCH 4/8] qemu: Rename virQEMUCapsObjectProps* -> virQEMUCapsDeviceProps*

[Andrea Bolognani]

In QOM, all devices are objects, which makes the existing names
technically correct; however, not all objects are devices, and
soon we're going to start looking for object properties in
addition to device properties: the former need to go through a
different code path, so we need to be able to tell them apart.
Using more precise names is a good way to achieve that.

While renaming, hunks are also being moved around a bit: the
new grouping, too, will make things nicer once we start adding
support for object properties.

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_capabilities.c | 239 ++-
 src/qemu/qemu_monitor.c  |   4 +-
 src/qemu/qemu_monitor.h  |   2 +-
 src/qemu/qemu_monitor_json.c |   2 +-
 src/qemu/qemu_monitor_json.h |   2 +-
 5 files changed, 128 insertions(+), 121 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c70bd27f18..83ec8a67d5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1559,6 +1559,13 @@ struct virQEMUCapsStringFlags {
 int flag;
 };
 
+struct virQEMUCapsObjectTypeProps {
+const char *type;
+struct virQEMUCapsStringFlags *props;
+size_t nprops;
+int capsCondition;
+};
+
 
 struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
 { "system_wakeup", QEMU_CAPS_WAKEUP },
@@ -1698,14 +1705,21 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] 
= {
 { "pl011", QEMU_CAPS_DEVICE_PL011 },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
+/* Device properties.
+ *
+ * The following can be probed either using the device-list-properties
+ * QMP command or, for older QEMU versions, from the help text obtained
+ * through the '-device xxx,?' command line option
+ */
+
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
 { "deflate-on-oom", QEMU_CAPS_VIRTIO_BALLOON_AUTODEFLATE },
 { "disable-legacy", QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY },
 { "iommu_platform", QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM },
 { "ats", QEMU_CAPS_VIRTIO_PCI_ATS },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBlk[] = {
 { "multifunction", QEMU_CAPS_PCI_MULTIFUNCTION },
 { "bootindex", QEMU_CAPS_BOOTINDEX },
 { "ioeventfd", QEMU_CAPS_VIRTIO_IOEVENTFD },
@@ -1719,7 +1733,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsVirtioBlk[] = {
 { "ats", QEMU_CAPS_VIRTIO_PCI_ATS },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioNet[] = {
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioNet[] = {
 { "tx", QEMU_CAPS_VIRTIO_TX_ALG },
 { "event_idx", QEMU_CAPS_VIRTIO_NET_EVENT_IDX },
 { "rx_queue_size", QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE },
@@ -1730,87 +1744,87 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsVirtioNet[] = {
 { "ats", QEMU_CAPS_VIRTIO_PCI_ATS },
 };
 
-static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsSpaprPCIHostBridge[] = {
+static struct virQEMUCapsStringFlags 
virQEMUCapsDevicePropsSpaprPCIHostBridge[] = {
 { "numa_node", QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioSCSI[] = {
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioSCSI[] = {
 { "iothread", QEMU_CAPS_VIRTIO_SCSI_IOTHREAD },
 { "disable-legacy", QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY },
 { "iommu_platform", QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM },
 { "ats", QEMU_CAPS_VIRTIO_PCI_ATS },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsPCIAssign[] = {
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsPCIAssign[] = {
 { "configfd", QEMU_CAPS_PCI_CONFIGFD },
 { "bootindex", QEMU_CAPS_PCI_BOOTINDEX },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVfioPCI[] = {
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVfioPCI[] = {
 { "bootindex", QEMU_CAPS_VFIO_PCI_BOOTINDEX },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsSCSIDisk[] = {
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsSCSIDisk[] = {
 { "channel", QEMU_CAPS_SCSI_DISK_CHANNEL },
 { "wwn", QEMU_CAPS_SCSI_DISK_WWN },
 { "share-rw", QEMU_CAPS_DISK_SHARE_RW },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIDEDrive[] = {
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsIDEDrive[] = {
 { "wwn", QEMU_CAPS_IDE_DRIVE_WWN },
 { "share-rw", QEMU_CAPS_DISK_SHARE_RW },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsPiix4PM[] = {
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsPiix4PM[] = {
 { "disable_s3", QEMU_CAPS_PIIX_DISABLE_S3 },
 { "disable_s4", QEMU_CAPS_PIIX_DISABLE_S4 },
 };
 
-static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUSBRedir[] = {

[libvirt] [PATCH 1/8] tests: Rename pseries-features-hpt test

[Andrea Bolognani]

We're going to use the same test case to exercise all optional
pSeries features, so a more generic name is needed.

Signed-off-by: Andrea Bolognani 
---
 .../{pseries-features-hpt.args => pseries-features.args}  | 0
 .../{pseries-features-hpt.xml => pseries-features.xml}| 0
 tests/qemuxml2argvtest.c  | 4 ++--
 tests/qemuxml2xmloutdata/pseries-features-hpt.xml | 1 -
 tests/qemuxml2xmloutdata/pseries-features.xml | 1 +
 tests/qemuxml2xmltest.c   | 2 +-
 6 files changed, 4 insertions(+), 4 deletions(-)
 rename tests/qemuxml2argvdata/{pseries-features-hpt.args => 
pseries-features.args} (100%)
 rename tests/qemuxml2argvdata/{pseries-features-hpt.xml => 
pseries-features.xml} (100%)
 delete mode 12 tests/qemuxml2xmloutdata/pseries-features-hpt.xml
 create mode 12 tests/qemuxml2xmloutdata/pseries-features.xml

diff --git a/tests/qemuxml2argvdata/pseries-features-hpt.args 
b/tests/qemuxml2argvdata/pseries-features.args
similarity index 100%
rename from tests/qemuxml2argvdata/pseries-features-hpt.args
rename to tests/qemuxml2argvdata/pseries-features.args
diff --git a/tests/qemuxml2argvdata/pseries-features-hpt.xml 
b/tests/qemuxml2argvdata/pseries-features.xml
similarity index 100%
rename from tests/qemuxml2argvdata/pseries-features-hpt.xml
rename to tests/qemuxml2argvdata/pseries-features.xml
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 688846b9b4..ca53912ebe 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1896,10 +1896,10 @@ mymain(void)
 QEMU_CAPS_VIRTIO_SCSI,
 QEMU_CAPS_DEVICE_VFIO_PCI);
 
-DO_TEST("pseries-features-hpt",
+DO_TEST("pseries-features",
 QEMU_CAPS_MACHINE_OPT,
 QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
-DO_TEST_FAILURE("pseries-features-hpt",
+DO_TEST_FAILURE("pseries-features",
 QEMU_CAPS_MACHINE_OPT);
 DO_TEST_PARSE_ERROR("pseries-features-invalid-machine", NONE);
 
diff --git a/tests/qemuxml2xmloutdata/pseries-features-hpt.xml 
b/tests/qemuxml2xmloutdata/pseries-features-hpt.xml
deleted file mode 12
index bcaf2e6fec..00
--- a/tests/qemuxml2xmloutdata/pseries-features-hpt.xml
+++ /dev/null
@@ -1 +0,0 @@
-../qemuxml2argvdata/pseries-features-hpt.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmloutdata/pseries-features.xml 
b/tests/qemuxml2xmloutdata/pseries-features.xml
new file mode 12
index 00..1b01dbace6
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/pseries-features.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/pseries-features.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0eb9e6c77a..a363b55446 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -763,7 +763,7 @@ mymain(void)
 QEMU_CAPS_VIRTIO_SCSI,
 QEMU_CAPS_DEVICE_VFIO_PCI);
 
-DO_TEST("pseries-features-hpt",
+DO_TEST("pseries-features",
 QEMU_CAPS_MACHINE_OPT,
 QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
 
-- 
2.14.3

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

[libvirt] [PATCH 5/8] qemu: Extract generic part from qemuMonitorJSONGetDeviceProps()

[Andrea Bolognani]

Querying properties for devices and objects is identical except
for the specific QMP command that needs to be called.

Take the existing qemuMonitorJSONGetDeviceProps(), scrub all the
device-specific parts, thus turning it into the generic helper
qemuMonitorJSONGetProps(), which is then used to reimplement the
original function and will be used again once object properties
are needed.

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_monitor_json.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index eb32811cd1..141873a705 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6058,9 +6058,11 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
 #undef MAKE_SET_CMD
 
 
-int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
-  const char *type,
-  char ***props)
+static int
+qemuMonitorJSONGetProps(qemuMonitorPtr mon,
+const char *type,
+char ***props,
+const char *impl)
 {
 int ret = -1;
 virJSONValuePtr cmd;
@@ -6072,7 +6074,7 @@ int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
 
 *props = NULL;
 
-if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
+if (!(cmd = qemuMonitorJSONMakeCommand(impl,
"s:typename", type,
NULL)))
 return -1;
@@ -6090,8 +6092,9 @@ int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
 
 if (!(data = virJSONValueObjectGetArray(reply, "return")) ||
 (n = virJSONValueArraySize(data)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("device-list-properties reply data was not an 
array"));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("%s reply data was not an array"),
+   impl);
 goto cleanup;
 }
 
@@ -6104,8 +6107,9 @@ int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
 const char *tmp;
 
 if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("device-list-properties reply data was missing 
'name'"));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("%s reply data was missing 'name'"),
+   impl);
 goto cleanup;
 }
 
@@ -6125,6 +6129,16 @@ int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
 }
 
 
+int
+qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
+  const char *type,
+  char ***props)
+{
+return qemuMonitorJSONGetProps(mon, type, props,
+   "device-list-properties");
+}
+
+
 char *
 qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon)
 {
-- 
2.14.3

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

Re: [libvirt] [Qemu-devel] [RFC] Defining firmware (OVMF, et al) metadata format & file

[Gerd Hoffmann]

  Hi,

> For OVMF (x86), I guess the initial set of properties should come from
> the "-D FOO[=BAR]" build flags that OVMF currently supports. (The list
> might grow or change incompatibly over time, so this is just a raw
> starter idea.)

> (0) ARCH (one of IA32, IA32X64, X64) -- the bitnesses of the PEI and DXE
> phases of the firmware.
> 
> IA32 stands for "32-bit PEI and DXE". Such firmware is usable for
> booting 32-bit OSes only, and runs on both qemu-system-i386 and
> qemu-system-x86_64.
> 
> IA32X64 stands for "32-bit PEI, 64-bit DXE". Needs qemu-system-x86_64
> and runs 64-bit OSes only.
> 
> X64 stands for "64-bit PEI and DXE". Needs qemu-system-x86_64 and runs
> 64-bit OSes only.

I'd simply put that info the 'arch' bucket.  IA32 gets 'i386' (assuming
we simply use qemu target names), IA32X64 + X64 get 'x86_64'.

> (1) SECURE_BOOT_ENABLE (boolean) -- whether the Secure Boot UEFI feature
> is built into the firmware image.

Probably useful for upper management (above libvirt), for enabling
secure boot key enrollment etc. in the UI.

> (2) SMM_REQUIRE (boolean) -- whether the SMM driver stack is included in
> the firmware.

Needed (libvirt must enable smm).

> (3) HTTP_BOOT_ENABLE (boolean) -- whether UEFI HTTP boot is available in
> the firmware image, in addition to the default PXE boot.

> (4) TLS_ENABLE (boolean) -- configurable independently of
> HTTP_BOOT_ENABLE, but only really makes sense in combination.

> (5) NETWORK_IP6_ENABLE (boolean) -- determines whether IPv6 support is
> available.

Do we actually need formal flags for these?  Which application would
use them?

For humans this can go into the freeform "description" text field.

> (6) FD_SIZE_IN_KB (one of: 1024, 2048, 4096) -- the size of the combined
> firmware image (executable portion and variable store together), in KB.

Hmm, dunno.  Might be useful as varstores are not portable between
builds of different sizes, so libvirt could use this for sanity checks.

> (7) DEBUG_ON_SERIAL_PORT (boolean) -- whether OVMF sends its debug
> messages to the QEMU debug IO port, or to the serial port.

> (8) SOURCE_DEBUG_ENABLE (boolean) -- whether OVMF includes the edk2
> debug agent that allows it to be debugged from a proprietary debugger
> program, likely connected via the emulated serial port.

> (9) CSM_ENABLE (boolean) -- whether OVMF includes traditional BIOS
> support, by including the SeaBIOS Compatibility Support Module.

Same question as for 3+4+5: Do we really need those?

> Given that the SeaBIOS CSM itself can be built with various
> configurations, it might make sense to list further properties when this
> property is enabled.

I doubt this is useful, established practice is that people just use
seabios directly instead if using the ovmf+csm combo.

> (10) E1000_ENABLE (boolean) -- whether OVMF includes the
> non-redistributable, binary only E1000(E) UEFI driver module from Intel
> (previously known as "PROEFI", more recently known as "BootUtil").

> (11) USE_OLD_SHELL (boolean) -- whether the UEFI shell implementation
> built into OVMF is the old (EDK1 / EFI-1 style) shell which lives in a
> separate repository, or the new (EDK2 / UEFI-2 style) shell which lives
> within the edk2 project.

> (12) TOOLCHAIN (string): the edk2 toolchain identifier with which the
> firmware was built.

> (13) TARGET (one of NOOPT, DEBUG, RELEASE) -- the target for which the
> firmware image was built.

Same question as for 3+4+5+7+8+9: Do we really need those?

> For ArmVirt, we have:
> 
> (12) ARCH (one of ARM and AARCH64) -- 32-bit / 64-bit distinction.

Likewise just use arch.

> (17) Well, what do I call this, let's call it ENTRY_POINT (one of Qemu,
> QemuKernel, Xen).
> 
> This distinguishes what VMM the ArmVirt firmware was built for, and for
> QEMU, it also distinguishes "boot from flash" (Qemu) or "boot as payload
> for another, earlier boot firmware" (QemuKernel).

Hmm, that looks simliar to u-boot, which I think can also be build as
both flash image and kernel image (loadable via -kernel).  So I guess we
want that one, as how-to-load-this-blob option (-bios vs -pflash vs -kernel).

cheers,
  Gerd

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

[libvirt] [PATCH] qemu: Add qemu functions for storage source private data handling

[Peter Krempa]

The qemu driver registered the helpers from util code, but it will be
necessary to format also some qemu-specific data.

Signed-off-by: Peter Krempa 
---

This patch conflicts with Michal's persistent reservations series. I'll
need the code in the place this patch places it later so I'm sending
this to avoid having to move this again very soon.

 src/qemu/qemu_domain.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b55013de6a..9802dcdbc6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1951,6 +1951,28 @@ qemuDomainObjPrivateFree(void *data)
 }


+static int
+qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
+  virStorageSourcePtr src)
+{
+if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0)
+return -1;
+
+return 0;
+}
+
+
+static int
+qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src,
+   virBufferPtr buf)
+{
+if (virStorageSourcePrivateDataFormatRelPath(src, buf) < 0)
+return -1;
+
+return 0;
+}
+
+
 static void
 qemuDomainObjPrivateXMLFormatVcpus(virBufferPtr buf,
virDomainDefPtr def)
@@ -2538,8 +2560,8 @@ virDomainXMLPrivateDataCallbacks 
virQEMUDriverPrivateDataCallbacks = {
 .chrSourceNew = qemuDomainChrSourcePrivateNew,
 .parse = qemuDomainObjPrivateXMLParse,
 .format = qemuDomainObjPrivateXMLFormat,
-.storageParse = virStorageSourcePrivateDataParseRelPath,
-.storageFormat = virStorageSourcePrivateDataFormatRelPath,
+.storageParse = qemuStorageSourcePrivateDataParse,
+.storageFormat = qemuStorageSourcePrivateDataFormat,
 };


-- 
2.16.2

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

[libvirt] [PATCH] m4: use pkgconfig to detect xenstore

[Olaf Hering]

Since Xen 4.9 a pkgconfig file exists to gather info about building
against libxenstore.so. Use it if available.

Signed-off-by: Olaf Hering 
---
 m4/virt-driver-xen.m4 | 41 +
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/m4/virt-driver-xen.m4 b/m4/virt-driver-xen.m4
index 2a26863ca..52d4f5162 100644
--- a/m4/virt-driver-xen.m4
+++ b/m4/virt-driver-xen.m4
@@ -27,25 +27,42 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_XEN], [
   old_CFLAGS="$CFLAGS"
   XEN_LIBS=""
   XEN_CFLAGS=""
+  fail=0
 
   dnl search for the Xen store library
+  dnl Either use the provided path, or make use of pkgconfig
+  dnl Xen versions prior 4.9 had no pkgconfig file
   if test "$with_xen" != "no" ; then
+xen_path_provided="no"
 if test "$with_xen" != "yes" && test "$with_xen" != "check" ; then
   XEN_CFLAGS="-I$with_xen/include"
   XEN_LIBS="-L$with_xen/lib64 -L$with_xen/lib"
+  xen_path_provided="yes"
+fi
+
+if test "$xen_path_provided" = "no" ; then
+  PKG_CHECK_MODULES([XEN], [xenstore], [
+  fail=0
+  with_xen=yes
+], [
+  fail=1
+])
+fi
+dnl manual check if either path was provided or pkgconfig does not exist
+if test "$xen_path_provided" = "yes" || test "$fail" = 1 ; then
+  CFLAGS="$CFLAGS $XEN_CFLAGS"
+  LIBS="$LIBS $XEN_LIBS"
+  AC_CHECK_LIB([xenstore], [xs_read], [
+ fail=0
+ with_xen=yes
+ XEN_LIBS="$XEN_LIBS -lxenstore"
+ ],[
+ if test "$with_xen" = "yes"; then
+   fail=1
+ fi
+ with_xen=no
+ ])
 fi
-fail=0
-CFLAGS="$CFLAGS $XEN_CFLAGS"
-LIBS="$LIBS $XEN_LIBS"
-AC_CHECK_LIB([xenstore], [xs_read], [
-   with_xen=yes
-   XEN_LIBS="$XEN_LIBS -lxenstore"
-   ],[
-   if test "$with_xen" = "yes"; then
- fail=1
-   fi
-   with_xen=no
-   ])
   fi
 
   if test "$with_xen" != "no" ; then

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

Re: [libvirt] [PATCH v2 06/12] qemu: Generate cmd line at startup

[Michal Privoznik]

On 03/08/2018 01:18 AM, John Ferlan wrote:
> 
> 
> On 03/06/2018 12:31 PM, Michal Privoznik wrote:
>> On 03/02/2018 02:22 PM, John Ferlan wrote:
>>>
>>>
>>> On 02/21/2018 01:11 PM, Michal Privoznik wrote:


>>> What happens when there's more than one disk using the managed mode
>>> where you have a "static" alias and path, wouldn't there be multiple
>>> lines with:
>>>
>>> -object
>>> pr-manager-helper,id=pr-helper0,path=/tmp/lib/domain--1-QEMUGuest1/pr-helper0.sock
>>
>> Ah sure.
>>
> 
> So having multiple "id=pr-helper0" is OK by QEMU?  OK, then.

No it's not. I've fixed this locally and I'll post v3 soon.

Michal

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

[libvirt] [PATCH 0/3] Couple of user alias fixes

[Michal Privoznik]

The deeper into the rabbit hole we go the more I think it was a
mistake to let users specify device aliases.

Michal Privoznik (3):
  virDomainDeviceValidateAliasForHotplug: Use correct domain defintion
  conf: Check for user aliases duplicates only
  qemu: Forbid user aliases for USB controllers

 src/conf/domain_conf.c  |  6 +++---
 src/qemu/qemu_domain.c  | 20 +++-
 tests/qemuxml2argvdata/user-aliases-usb.xml | 19 +++
 tests/qemuxml2argvdata/user-aliases.xml |  1 -
 tests/qemuxml2argvtest.c|  1 +
 5 files changed, 42 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/user-aliases-usb.xml

-- 
2.16.1

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

[libvirt] [PATCH 2/3] conf: Check for user aliases duplicates only

[Michal Privoznik]

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

When validating a device XML config we check if user provided
alias is unique. We do this by maintaining a hash table of device
aliases as we iterated over all devices defined for the domain.
However, it may happen that what appears as two devices in domain
XML is in fact just one interface in hypervisor. For instance in
qemu driver this is true for uhci/ehci controllers. In that case
an error is reported even though it is not actually an error. At
any rate, we can assume libvirt generated aliases to be unique
and thus really check user provided ones only.

Signed-off-by: Michal Privoznik 
---
 src/conf/domain_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b98b1ca42..04a6ee77a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5573,7 +5573,7 @@ virDomainDeviceDefValidateAliasesIterator(virDomainDefPtr 
def,
 struct virDomainDefValidateAliasesData *data = opaque;
 const char *alias = info->alias;
 
-if (!alias)
+if (!alias || !virDomainDeviceAliasIsUserAlias(alias))
 return 0;
 
 /* Some crazy backcompat for consoles. */
-- 
2.16.1

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

[libvirt] [PATCH 3/3] qemu: Forbid user aliases for USB controllers

[Michal Privoznik]

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

These are bit different than other devices. Their alias also
specify the name of the bus. For instance, if there are these
'joined' USB devices:

  


  
  


  

which translates to cmd line as:

  -device ich9-usb-ehci1,id=ua-myUSB,bus=pci.0,addr=0x4.0x7
  -device 
ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4

The problem is that UHCI is still trying to serve 'usb.0' bus.
Rather than trying to come up with some complicated algorithm to
make everything work, lets forbid user aliases for USB
controllers.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_domain.c  | 20 +++-
 tests/qemuxml2argvdata/user-aliases-usb.xml | 19 +++
 tests/qemuxml2argvdata/user-aliases.xml |  1 -
 tests/qemuxml2argvtest.c|  1 +
 4 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/user-aliases-usb.xml

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b55013de6..67c145493 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4565,6 +4565,21 @@ qemuDomainDeviceDefValidateControllerSATA(const 
virDomainControllerDef *controll
 }
 
 
+static int
+qemuDomainDeviceDefValidateControllerUSB(const virDomainControllerDef 
*controller)
+{
+if (controller->info.alias &&
+virDomainDeviceAliasIsUserAlias(controller->info.alias)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("Providing user alias to USB "
+ "controllers is not supported yet"));
+return -1;
+}
+
+return 0;
+}
+
+
 static int
 qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
   const virDomainDef *def,
@@ -4602,10 +4617,13 @@ qemuDomainDeviceDefValidateController(const 
virDomainControllerDef *controller,
 qemuCaps);
 break;
 
+case VIR_DOMAIN_CONTROLLER_TYPE_USB:
+ret = qemuDomainDeviceDefValidateControllerUSB(controller);
+break;
+
 case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
 case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
 case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
-case VIR_DOMAIN_CONTROLLER_TYPE_USB:
 case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
 break;
 }
diff --git a/tests/qemuxml2argvdata/user-aliases-usb.xml 
b/tests/qemuxml2argvdata/user-aliases-usb.xml
new file mode 100644
index 0..6dade4572
--- /dev/null
+++ b/tests/qemuxml2argvdata/user-aliases-usb.xml
@@ -0,0 +1,19 @@
+
+  gentoo
+  a75aca4b-a02f-2bcb-4a91-c93cd848c34b
+  4194304
+  4194304
+  4
+  
+hvm
+
+
+  
+  
+/usr/bin/qemu-system-x86_64
+
+  
+  
+
+  
+
diff --git a/tests/qemuxml2argvdata/user-aliases.xml 
b/tests/qemuxml2argvdata/user-aliases.xml
index 52132a82d..6f1fed636 100644
--- a/tests/qemuxml2argvdata/user-aliases.xml
+++ b/tests/qemuxml2argvdata/user-aliases.xml
@@ -71,7 +71,6 @@
   
 
 
-  
   
 
 
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 688846b9b..5758811b8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2965,6 +2965,7 @@ mymain(void)
 QEMU_CAPS_DEVICE_ISA_SERIAL,
 QEMU_CAPS_HDA_DUPLEX);
 DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI);
+DO_TEST_PARSE_ERROR("user-aliases-usb", QEMU_CAPS_KVM);
 
 if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
 virFileDeleteTree(fakerootdir);
-- 
2.16.1

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

[libvirt] [PATCH 1/3] virDomainDeviceValidateAliasForHotplug: Use correct domain defintion

[Michal Privoznik]

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

For some weird reason this function is getting live and
persistent def for domain but then accesses vm->def and
vm->newDef directly. This is rather unsafe as we can be
accessing NULL pointer.

Signed-off-by: Michal Privoznik 
---
 src/conf/domain_conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 70b19311b..b98b1ca42 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5679,11 +5679,11 @@ virDomainDeviceValidateAliasForHotplug(virDomainObjPtr 
vm,
 return -1;
 
 if (persDef &&
-virDomainDeviceValidateAliasImpl(vm->def, dev) < 0)
+virDomainDeviceValidateAliasImpl(persDef, dev) < 0)
 return -1;
 
 if (liveDef &&
-virDomainDeviceValidateAliasImpl(vm->newDef, dev) < 0)
+virDomainDeviceValidateAliasImpl(liveDef, dev) < 0)
 return -1;
 
 return 0;
-- 
2.16.1

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

Re: [libvirt] [Qemu-devel] [RFC] Defining firmware (OVMF, et al) metadata format & file

[Kashyap Chamarthy]

On Thu, Mar 08, 2018 at 09:47:27PM +0100, Laszlo Ersek wrote:
> On 03/08/18 16:47, Daniel P. Berrangé wrote:
> > On Thu, Mar 08, 2018 at 12:10:30PM +0100, Laszlo Ersek wrote:

[...]

> >> For OVMF (x86), I guess the initial set of properties should come from
> >> the "-D FOO[=BAR]" build flags that OVMF currently supports. (The list
> >> might grow or change incompatibly over time, so this is just a raw
> >> starter idea.)
> > 
> > I really don't want to see us using firmware implementation specific
> > property names in these files. It means libvirt will require knowledge
> > of what each different firmware's property names mean.
> > 
> > We need to have some core standardized set of property names that can
> > be provided by any firmware implementation using the same terminology.
> > 
> > If we want to /also/ provide some extra firmeware-specific property
> > names that would be ok for informative purposes, but when lbivirt is
> > picking which firmware file to use, it would only ever look at the
> > standardized property names/values.
> 
> This is a reasonable requirement from the libvirt side.
> 
> Unfortunately (or not), it requires someone (or a tight group of people)
> to collect the features of all virtual firmwares in existence, and
> extract a common set of properties that maps back to each firmware one
> way or another. 

Hmm, if people consider the above worthwhile (no clue how much time &
investigation it takes to arrive at a common set of properties) maybe
slowly we should start collecting such a page?  From a quick look up,
list of open source firmware implementations I found so (besides OVMF &
ArmVirt):

  - OpenBIOS
  - SmartFirmware
  - OpenBoot
  - CoreBoot
  - U-Boot
  - SLOF
  - ...

Source: https://en.wikipedia.org/wiki/OpenBIOS

I notice you said "virtual firmwares".  I couldn't find such a list from
my look up.

Hmm, I also wonder if the "arriving at a common set of properties across
existing virtual firmwares" is an absolute blocker.

> This is not unusual (basically this is how all standards
> bodies work that intend to codify existing practice), it just needs a
> bunch of work and coordination. We'll have to maintain a registry.
> 
> Personally I can't comment on anything else than OVMF and the ArmVirt
> firmwares.
> 
> Thanks,
> Laszlo

-- 
/kashyap

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

Re: [libvirt] [RFC] Defining firmware (OVMF, et al) metadata format & file

[Kashyap Chamarthy]

On Thu, Mar 08, 2018 at 08:52:45AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > > [*] Open question: Who, between QEMU and libvirt, should define the said
> > > firmware metadata format and file?
> > 
> > IMHO QEMU should be defining the format, because the file will contain
> > info about certain QEMU features associated with the firmware (eg smm).
> > Also there are potentially other non-libvirt mgmt apps that spawn QEMU
> > which would like this info (eg libguestfs), so having libvirt define the
> > format is inappropriate.
> > 
> > I'd suggest we just need something in docs/specs/firmware-metadata.rst
> > for QEMU source tree.
> > 
> > Potentially QEMU could even use the metadata files itself for finding
> > the default firmeware images, instead of compiling this info into its
> > binaries. I wouldn't suggest we need todo that right away, but bear it
> > in mind as a potential use case.
> 
> With qemu using this itself in mind it probably makes sense to specify
> this as qapi schema.  That'll simplify parsing and using these files in
> qemu, and possibly simplifies things on the libvirt side too.

Yeah, FWIW, I too find using the QAPI schema for the metadata file
format more appealing (rather than a separate file format).  And
libvirt already has infrastructure to handle QAPI.

-- 
/kashyap

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