[libvirt] [PATCH 0/6] Couple of perf events APIs fixes

[Michal Privoznik]

I really like to see these in before release and thus APIs get
written in the stone.

One note though regarding the last patch: while writing it I've
noticed couple of other getter APIs don't grab any job at all. I
think they should grab _QUERY job though. So maybe those will
need some fixing too.

Michal Privoznik (6):
  remoteDomainGetPerfEvents: Re-indent
  virDomain{Get,Set}PerfEvents: Add @flags argument
  virsh: Prefer VIRSH_COMMON_OPT_DOMAIN_FULL over full enumeration
  virsh: Make perf accept event list separated by commas
  virDomain{Get,Set}PerfEvents: support --config --live --current
  virDomain{Get,Set}PerfEvents: Grab job

 daemon/remote.c  |  2 +-
 include/libvirt/libvirt-domain.h |  6 ++-
 src/driver-hypervisor.h  |  6 ++-
 src/libvirt-domain.c | 22 +
 src/qemu/qemu_driver.c   | 99 +---
 src/remote/remote_driver.c   | 12 +++--
 src/remote/remote_protocol.x |  2 +
 tools/virsh-domain.c | 27 +++
 tools/virsh.pod  | 25 ++
 9 files changed, 141 insertions(+), 60 deletions(-)

-- 
2.7.3

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

[libvirt] [PATCH 3/6] virsh: Prefer VIRSH_COMMON_OPT_DOMAIN_FULL over full enumeration

[Michal Privoznik]

We have a macro that does exactly what is done via full
enumeration.

Signed-off-by: Michal Privoznik 
---
 tools/virsh-domain.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a105268..a09cdec 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8551,11 +8551,7 @@ static const vshCmdInfo info_perf[] = {
 };
 
 static const vshCmdOptDef opts_perf[] = {
-{.name = "domain",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("domain name, id or uuid")
-},
+VIRSH_COMMON_OPT_DOMAIN_FULL,
 {.name = "enable",
  .type = VSH_OT_STRING,
  .help = N_("perf events which will be enabled")
-- 
2.7.3

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

[libvirt] [PATCH 4/6] virsh: Make perf accept event list separated by commas

[Michal Privoznik]

Everywhere else we use a comma separated list. There's no good
reason to make 'perf' command an exception. Currently, it accepts
string list separated by '|'.

Signed-off-by: Michal Privoznik 
---
 tools/virsh-domain.c |  2 +-
 tools/virsh.pod  | 17 +
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a09cdec..2dbb890 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8575,7 +8575,7 @@ virshParseEventStr(vshControl *ctl,
 size_t i, ntok;
 int ret = -1;
 
-if (!(tok = virStringSplitCount(event, "|", 0, )))
+if (!(tok = virStringSplitCount(event, ",", 0, )))
 return -1;
 
 if (ntok > VIR_PERF_EVENT_LAST) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 0c02d7f..a9915b0 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2144,11 +2144,11 @@ The guaranteed minimum memory allocation for the guest.
 
 Specifying -1 as a value for these limits is interpreted as unlimited.
 
-=item B I [I<--enable> B]
-[I<--disable> B]
+=item B I [I<--enable> B]
+[I<--disable> B]
 
 Get the current perf events setting or enable/disable specific perf
-event for a guest domain.
+events for a guest domain.
 
 Perf is a performance analyzing tool in Linux, and it can instrument
 CPU performance counters, tracepoints, kprobes, and uprobes (dynamic
@@ -2158,11 +2158,12 @@ pure kernel counters, in this case they are called 
software events,
 including context-switches, minor-faults, etc.. Now dozens of events
 from different sources can be supported by perf.
 
-Currently only QEMU/KVM supports I<--enable> and I<--disable>.
-B is a string listing one or more events, in the format
-of name|name|name. Only "cmt" event is supported presently. CMT is
-a PQos (Platform Qos) feature to monitor the usage of cache by
-applications running on the platform.
+Currently only QEMU/KVM supports this command. The I<--enable> and I<--disable>
+option combined with B can be used to enabled or disable specific
+performance event. B is a string list of one or more events
+separated by commas. However, just "cmt" event is supported presently. CMT is a
+PQos (Platform Qos) feature to monitor the usage of cache by applications
+running on the platform.
 
 =item B I [I<--weight> B]
 [I<--device-weights> B]
-- 
2.7.3

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

[libvirt] [PATCH 6/6] virDomain{Get,Set}PerfEvents: Grab job

[Michal Privoznik]

Even though we have the machine locked throughout whole APIs we
are querying/modifying domain internal state. We should grab a
job whilst doing that.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 56120ff..7fef901 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10069,9 +10069,12 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
 if (virDomainSetPerfEventsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (virDomainObjGetDefs(vm, flags, , ) < 0)
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 
+if (virDomainObjGetDefs(vm, flags, , ) < 0)
+goto endjob;
+
 if (def) {
 for (i = 0; i < nparams; i++) {
 virTypedParameterPtr param = [i];
@@ -10079,16 +10082,16 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
 type = virPerfEventTypeFromString(param->field);
 
 if (!enabled && virPerfEventDisable(priv->perf, type))
-goto cleanup;
+goto endjob;
 if (enabled && virPerfEventEnable(priv->perf, type, vm->pid))
-goto cleanup;
+goto endjob;
 
 def->perf->events[type] = enabled ?
 VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
 }
 
 if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, 
driver->caps) < 0)
-goto cleanup;
+goto endjob;
 }
 
 if (persistentDef) {
@@ -10102,11 +10105,14 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
 }
 
 if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 
0)
-goto cleanup;
+goto endjob;
 }
 
 ret = 0;
 
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+
  cleanup:
 virDomainObjEndAPI();
 virObjectUnref(cfg);
@@ -10146,9 +10152,12 @@ qemuDomainGetPerfEvents(virDomainPtr dom,
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 goto cleanup;
 
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+goto cleanup;
+
 if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, ,
 ) < 0)
-goto cleanup;
+goto endjob;
 
 priv = vm->privateData;
 
@@ -10164,7 +10173,7 @@ qemuDomainGetPerfEvents(virDomainPtr dom,
  virPerfEventTypeToString(i),
  perf_enabled) < 0) {
 virTypedParamsFree(par, npar);
-goto cleanup;
+goto endjob;
 }
 }
 
@@ -10172,6 +10181,9 @@ qemuDomainGetPerfEvents(virDomainPtr dom,
 *nparams = npar;
 ret = 0;
 
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+
  cleanup:
 virDomainObjEndAPI();
 virObjectUnref(caps);
-- 
2.7.3

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

[libvirt] [PATCH 5/6] virDomain{Get, Set}PerfEvents: support --config --live --current

[Michal Privoznik]

Now that we have @flags we can support changing perf events just
in active or inactive configuration regardless of the other.
Previously, calling virDomainSetPerfEvents set events in both
active and inactive configuration at once. Even though we allow
users to set perf events that are to be enabled once domain is
started up. The virDomainGetPerfEvents API was flawed too. It
returned just runtime info.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 76 +++---
 tools/virsh-domain.c   | 14 ++
 tools/virsh.pod|  8 ++
 3 files changed, 75 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cbd520b..56120ff 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10054,7 +10054,8 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
 virPerfEventType type;
 bool enabled;
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
 
 if (virTypedParamsValidate(params, nparams, VIR_PERF_PARAMETERS) < 0)
 return -1;
@@ -10071,31 +10072,37 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
 if (virDomainObjGetDefs(vm, flags, , ) < 0)
 goto cleanup;
 
-for (i = 0; i < nparams; i++) {
-virTypedParameterPtr param = [i];
-enabled = params->value.b;
-type = virPerfEventTypeFromString(param->field);
+if (def) {
+for (i = 0; i < nparams; i++) {
+virTypedParameterPtr param = [i];
+enabled = params->value.b;
+type = virPerfEventTypeFromString(param->field);
 
-if (!enabled && virPerfEventDisable(priv->perf, type))
-goto cleanup;
-if (enabled && virPerfEventEnable(priv->perf, type, vm->pid))
-goto cleanup;
+if (!enabled && virPerfEventDisable(priv->perf, type))
+goto cleanup;
+if (enabled && virPerfEventEnable(priv->perf, type, vm->pid))
+goto cleanup;
 
-if (def) {
 def->perf->events[type] = enabled ?
 VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
-
-if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, 
driver->caps) < 0)
-goto cleanup;
 }
 
-if (persistentDef) {
+if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, 
driver->caps) < 0)
+goto cleanup;
+}
+
+if (persistentDef) {
+for (i = 0; i < nparams; i++) {
+virTypedParameterPtr param = [i];
+enabled = params->value.b;
+type = virPerfEventTypeFromString(param->field);
+
 persistentDef->perf->events[type] = enabled ?
 VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
-
-if (virDomainSaveConfig(cfg->configDir, driver->caps, 
persistentDef) < 0)
-goto cleanup;
 }
+
+if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 
0)
+goto cleanup;
 }
 
 ret = 0;
@@ -10112,28 +10119,50 @@ qemuDomainGetPerfEvents(virDomainPtr dom,
 int *nparams,
 unsigned int flags)
 {
-size_t i;
+virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr vm = NULL;
 qemuDomainObjPrivatePtr priv;
-int ret = -1;
+virDomainDefPtr persistentDef;
 virTypedParameterPtr par = NULL;
+virCapsPtr caps = NULL;
 int maxpar = 0;
 int npar = 0;
+size_t i;
+int ret = -1;
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG |
+  VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+/* We don't return strings, and thus trivially support this flag.  */
+flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
 
 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
 
-priv = vm->privateData;
-
 if (virDomainGetPerfEventsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
+if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+goto cleanup;
+
+if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, ,
+) < 0)
+goto cleanup;
+
+priv = vm->privateData;
+
 for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
+bool perf_enabled;
+
+if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+perf_enabled = persistentDef->perf->events[i] == 
VIR_TRISTATE_BOOL_YES;
+else
+perf_enabled = virPerfEventIsEnabled(priv->perf, i);
+
 if (virTypedParamsAddBoolean(, , ,
  virPerfEventTypeToString(i),
- virPerfEventIsEnabled(priv->perf, i)) < 
0) {
+ perf_enabled) < 0) {
 virTypedParamsFree(par, npar);
 goto cleanup;
 }
@@ -10145,6 +10174,7 @@ 

[libvirt] [PATCH 2/6] virDomain{Get, Set}PerfEvents: Add @flags argument

[Michal Privoznik]

I've noticed that these APIs are missing @flags argument. Even
though we don't have a use for them, it's our policy that every
new API must have @flags.

Signed-off-by: Michal Privoznik 
---
 daemon/remote.c  |  2 +-
 include/libvirt/libvirt-domain.h |  6 --
 src/driver-hypervisor.h  |  6 --
 src/libvirt-domain.c | 22 ++
 src/qemu/qemu_driver.c   | 11 ---
 src/remote/remote_driver.c   |  4 +++-
 src/remote/remote_protocol.x |  2 ++
 tools/virsh-domain.c |  5 +++--
 8 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index bbe22b0..9db93ff 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2694,7 +2694,7 @@ remoteDispatchDomainGetPerfEvents(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
 goto cleanup;
 
-if (virDomainGetPerfEvents(dom, , ) < 0)
+if (virDomainGetPerfEvents(dom, , , args->flags) < 0)
 goto cleanup;
 
 if (nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) {
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 552a40b..728b6eb 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1848,10 +1848,12 @@ void 
virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
 
 int virDomainGetPerfEvents(virDomainPtr dom,
virTypedParameterPtr *params,
-   int *nparams);
+   int *nparams,
+   unsigned int flags);
 int virDomainSetPerfEvents(virDomainPtr dom,
virTypedParameterPtr params,
-   int nparams);
+   int nparams,
+   unsigned int flags);
 
 /*
  * BlockJob API
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index d0e7298..d11ff7f 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -964,12 +964,14 @@ typedef int
 typedef int
 (*virDrvDomainGetPerfEvents)(virDomainPtr dom,
  virTypedParameterPtr *params,
- int *nparams);
+ int *nparams,
+ unsigned int flags);
 
 typedef int
 (*virDrvDomainSetPerfEvents)(virDomainPtr dom,
  virTypedParameterPtr params,
- int nparams);
+ int nparams,
+ unsigned int flags);
 
 typedef int
 (*virDrvDomainBlockJobAbort)(virDomainPtr dom,
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 42031bc..3e144b6 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9695,6 +9695,7 @@ virDomainOpenChannel(virDomainPtr dom,
  * @domain: a domain object
  * @params: where to store perf events setting
  * @nparams: number of items in @params
+ * @flags: extra flags; not used yet, so callers should always pass 0
  *
  * Get all perf events setting. Possible fields returned in @params are
  * defined by VIR_DOMAIN_PERF_* macros and new fields will likely be
@@ -9704,12 +9705,13 @@ virDomainOpenChannel(virDomainPtr dom,
  */
 int virDomainGetPerfEvents(virDomainPtr domain,
virTypedParameterPtr *params,
-   int *nparams)
+   int *nparams,
+   unsigned int flags)
 {
 virConnectPtr conn;
 
-VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p",
- params, nparams);
+VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=%x",
+ params, nparams, flags);
 
 virResetLastError();
 
@@ -9721,7 +9723,8 @@ int virDomainGetPerfEvents(virDomainPtr domain,
 
 if (conn->driver->domainGetPerfEvents) {
 int ret;
-ret = conn->driver->domainGetPerfEvents(domain, params, nparams);
+ret = conn->driver->domainGetPerfEvents(domain, params,
+nparams, flags);
 if (ret < 0)
 goto error;
 return ret;
@@ -9740,6 +9743,7 @@ int virDomainGetPerfEvents(virDomainPtr domain,
  * @params: pointer to perf events parameter object
  * @nparams: number of perf event parameters (this value can be the same
  *   less than the number of parameters supported)
+ * @flags: extra flags; not used yet, so callers should always pass 0
  *
  * Enable or disable the particular list of perf events you care about.
  *
@@ -9747,12 +9751,13 @@ int virDomainGetPerfEvents(virDomainPtr domain,
  */
 int virDomainSetPerfEvents(virDomainPtr domain,
virTypedParameterPtr params,
-   int nparams)
+   int nparams,
+   unsigned int flags)
 {
 virConnectPtr conn;
 
-VIR_DOMAIN_DEBUG(domain, 

[libvirt] [PATCH 1/6] remoteDomainGetPerfEvents: Re-indent

[Michal Privoznik]

There are few lines off the indentation.

Signed-off-by: Michal Privoznik 
---
 src/remote/remote_driver.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b3c7b9c..254e0e9 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1837,10 +1837,10 @@ remoteDomainGetPerfEvents(virDomainPtr domain,
 goto done;
 
 if (virTypedParamsDeserialize((virTypedParameterRemotePtr) 
ret.params.params_val,
- ret.params.params_len,
- REMOTE_DOMAIN_PERF_EVENTS_MAX,
- params,
- nparams) < 0)
+  ret.params.params_len,
+  REMOTE_DOMAIN_PERF_EVENTS_MAX,
+  params,
+  nparams) < 0)
 goto cleanup;
 
 rv = 0;
-- 
2.7.3

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

Re: [libvirt] [PATCH V4 4/6] libxl: support creating domain with VF assignment from a pool

[Jim Fehlig]

On 03/30/2016 03:38 AM, Chun Yan Liu wrote:
>
 On 3/29/2016 at 08:05 AM, in message <56f9c6d0.5000...@suse.com>, Jim 
 Fehlig
>  wrote: 
>> On 03/21/2016 02:11 AM, Chunyan Liu wrote: 
>>> Add codes to support creating domain with network defition of assigning 
>>> SRIOV VF from a pool. 
>>>
>>> Signed-off-by: Chunyan Liu  
>>> --- 
>>>  src/libxl/libxl_domain.c | 50  
>>  
>>>  tests/Makefile.am|  3 +++ 
>>>  2 files changed, 53 insertions(+) 
>>>
>>> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c 
>>> index c8d09b1..d11bf3a 100644 
>>> --- a/src/libxl/libxl_domain.c 
>>> +++ b/src/libxl/libxl_domain.c 
>>> @@ -36,6 +36,7 @@ 
>>>  #include "virtime.h" 
>>>  #include "locking/domain_lock.h" 
>>>  #include "xen_common.h" 
>>> +#include "network/bridge_driver.h" 
>>>   
>>>  #define VIR_FROM_THIS VIR_FROM_LIBXL 
>>>   
>>> @@ -764,6 +765,10 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, 
>>>  if (net->ifname && 
>>>  STRPREFIX(net->ifname, LIBXL_GENERATED_PREFIX_XEN)) 
>>>  VIR_FREE(net->ifname); 
>>> + 
>>> +/* cleanup actual device */ 
>>> +virDomainNetRemoveHostdev(vm->def, net); 
>>> +networkReleaseActualDevice(vm->def, net); 
>>>  } 
>>>  } 
>>>   
>>> @@ -960,6 +965,48 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def,  
>> libxl_domain_config *d_config) 
>>>  } 
>>>  } 
>>>   
>>> +static int 
>>> +libxlNetworkPrepareDevices(virDomainDefPtr def) 
>>> +{ 
>>> +int ret = -1; 
>>> +size_t i; 
>>> + 
>>> +for (i = 0; i < def->nnets; i++) { 
>>> +virDomainNetDefPtr net = def->nets[i]; 
>>> +int actualType; 
>>> + 
>>> +/* If appropriate, grab a physical device from the configured 
>>> + * network's pool of devices, or resolve bridge device name 
>>> + * to the one defined in the network definition. 
>>> + */ 
>>> +if (networkAllocateActualDevice(def, net) < 0) 
>>> +goto cleanup; 
>>> + 
>>> +actualType = virDomainNetGetActualType(net); 
>>> +if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV && 
>>> +net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { 
>>> +/* Each type='hostdev' network device must also have a 
>>> + * corresponding entry in the hostdevs array. For netdevs 
>>> + * that are hardcoded as type='hostdev', this is already 
>>> + * done by the parser, but for those allocated from a 
>>> + * network / determined at runtime, we need to do it 
>>> + * separately. 
>>> + */ 
>>> +virDomainHostdevDefPtr hostdev =  
>> virDomainNetGetActualHostdev(net); 
>>> +virDomainHostdevSubsysPCIPtr pcisrc =  
>> >source.subsys.u.pci; 
>>> + 
>>> +if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && 
>>> +hostdev->source.subsys.type ==  
>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) 
>>> +pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; 
>>> + 
>>> +if (virDomainHostdevInsert(def, hostdev) < 0) 
>>> +goto cleanup; 
>>> +} 
>>> +} 
>>> +ret = 0; 
>>> + cleanup: 
>>> +return ret; 
>>  
>> Nothing to cleanup here. I think we should just return -1 on failure paths  
>> and 0 
>> on success. 
>>  
>>> +} 
>>>   
>>>  /* 
>>>   * Start a domain through libxenlight. 
>>> @@ -1036,6 +1083,9 @@ libxlDomainStart(libxlDriverPrivatePtr driver,  
>> virDomainObjPtr vm, 
>>>  vm, true) < 0) 
>>>  goto cleanup; 
>>>   
>>> +if (libxlNetworkPrepareDevices(vm->def) < 0) 
>>> +goto cleanup; 
>>  
>> I accessed a machine to test this series and have found a few problems. 
>>  
>> 1. When attaching an SR-IOV vf from a pool, the attach appears successful. 
>>I see that xen's pciback driver is bound to the device in the host. I 
>>didn't see the device in the guest (could be a guest problem), 
> For pv guest, from testing, it is shown in guest. For HVM, seems
> xl pci-attach has the same result.

Yes, I'm seeing the same thing. Looks like a bug in libxl or qemu.

>> nor in 
>>dumpxml or /var/run/libvirt/libxl/.xml.
> Ah, there is a mistake during a review change to original patch 1/6.
> Didn't notice that earlier.
>
> In libxlDomainAttachNetDevice:
> The if (!ret) is needed, maybe cleanup is not proper.
> Since for actual type is hostdev case, after a successful
> libxlDomainAttachHostDevice, we need to
> update vm->def->nets too.
>
>   cleanup:
>  libxl_device_nic_dispose();
> - out:
> -if (!ret)
> +if (!ret) {
>  vm->def->nets[vm->def->nnets++] = net;
> +} else {
> +virDomainNetRemoveHostdev(vm->def, net);
> +networkReleaseActualDevice(vm->def, net);
> +}
>
> Similar for libxDomainDetachNetDevice cleanup:
> Original 

[libvirt] [For 1.3.3 1/2] libxl: fix attaching net device of type hostdev

[Jim Fehlig]

Chunyan sent a correct patch to fix a resource leak on error in
libxlDomainAttachNetDevice

https://www.redhat.com/archives/libvir-list/2016-March/msg00924.html

I made what was thought to be an improvement and pushed the patch as
commit e6336442. As it turns out, my change broke adding net devices
that are actually hostdevs to the list of nets in virDomainDef. This
patch changes e6336442 to resemble Chunyan's original, correct
patch.

Signed-off-by: Jim Fehlig 
---
 src/libxl/libxl_driver.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 5103495..9955cc7 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3174,12 +3174,13 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
 goto cleanup;
 }
 
-vm->def->nets[vm->def->nnets++] = net;
 ret = 0;
 
  cleanup:
 libxl_device_nic_dispose();
-if (ret) {
+if (!ret) {
+vm->def->nets[vm->def->nnets++] = net;
+} else {
 virDomainNetRemoveHostdev(vm->def, net);
 networkReleaseActualDevice(vm->def, net);
 }
-- 
2.6.1

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

[libvirt] [For 1.3.3 0/2] small fixes for recent libxl commits

[Jim Fehlig]

For 1.3.3: small fixes for recent libxl commits

Chunyan noted some problems in commits e6336442 and b5534e53

https://www.redhat.com/archives/libvir-list/2016-March/msg01431.html

These two patches fix those issues, essentially making e6336442 and
b5534e53 resemble the correct patches originally submitted by Chunyan.
See the individual patches for details.

With these fixes, Chunyan's patch to "support creating domain with VF
assignment from a pool" [0] works fine, save a bug or two in xen-unstable
libxl. The patch has been through several revisions and on the list for
quite some time. Would it be okay to commit it, along with these patches,
for 1.3.3?

[0] https://www.redhat.com/archives/libvir-list/2016-March/msg00930.html

Jim Fehlig (2):
  libxl: fix attaching net device of type hostdev
  libxl: fix net device detach

 src/libxl/libxl_driver.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.6.1

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

[libvirt] [For 1.3.3 2/2] libxl: fix net device detach

[Jim Fehlig]

Chunyan sent a nice cleanup patch for libxlDomainDetachNetDevice

https://www.redhat.com/archives/libvir-list/2016-March/msg00926.html

which I incorrectly modified before pushing as commit b5534e53. My
modification caused network devices of type hostdev to no longer
be removed. This patch changes b5534e53 to resemble Chunyan's
original, correct patch.

Signed-off-by: Jim Fehlig 
---
 src/libxl/libxl_driver.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9955cc7..bf97c9c 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3449,12 +3449,14 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver,
 goto cleanup;
 }
 
-networkReleaseActualDevice(vm->def, detach);
-virDomainNetRemove(vm->def, detachidx);
 ret = 0;
 
  cleanup:
 libxl_device_nic_dispose();
+if (!ret) {
+networkReleaseActualDevice(vm->def, detach);
+virDomainNetRemove(vm->def, detachidx);
+}
 virObjectUnref(cfg);
 return ret;
 }
-- 
2.6.1

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

Re: [libvirt] [PATCH] migration: convert speed from Mb/sec to bytes/sec in drive-mirror jobs

[John Ferlan]

On 03/28/2016 12:02 AM, Rudy Zhang wrote:
> Block migration speed is differect from memory migration speed, because
> it not convert speed from Mb/sec to bytes/sec in the drive-mirror job.
> 

"different"

Perhaps better stated:

Commit id '7b7600b3' added qemuMigrationDriveMirror to handle NBD
mirroring, but passed the migrate_speed listed in MiB/s to be used for
the mirror_speed which expects a bytes/s value.

This patch will convert the migrate_speed value to its mirror_speed
equivalent.

> Signed-off-by: Rudy Zhang 
> ---
>  src/qemu/qemu_migration.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Not my area of expertise, but since it was sitting here waiting for
review. Had to dig a bit, but yes I see the passed 'speed' is the
migration speed which yes, is in MiB/s...

What I didn't dig on was whether the migrate bandwidth using a negative
value (eg essentially unlimited) is passed around as a -1 or some other
theoretically maximum. If it is, then the bit shift done here is going
to have a problem as the value passed to qemuMonitorDriveMirror won't be
right (look at the qemuDomainBlockRebase and see how it limits things).

> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 8bc76bf..7648d8c 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2135,8 +2135,8 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,

My suggested adjustments:

1. Adjust name of parameter "speed" to "migrate_speed" and also comments
into function to indicate new name and that it's MiB/s.

2. Add a local "unsigned long long mirror_speed = migrate_speed;"

3. Look at qemuDomainBlockRebase and how it checks to make sure the
speed will fit when shifted... Not sure we want to error out at this
point, but some thing like:

if (mirror_speed > LLONG_MAX >> 20)
mirror_speed = 0;  /* means unlimited */
else
mirror_speed <<= 20;

>  
>  qemuBlockJobSyncBegin(disk);
>  /* Force "raw" format for NBD export */
> -mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest,
> - "raw", speed, 0, 0, mirror_flags);

4. Change the following to use "mirror_speed", I think for alignment
stuff it'll be best to be:

 mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest, "raw",
  mirror_speed, 0, 0, mirror_flags);


Of course I could be totally off base and we just choose to use
mirror_speed = 0 and forget all the adjustments and we won't need to
pass migrate_speed either!

John
> +mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, 
> nbd_dest,"raw",
> +(unsigned long long)speed << 20, 0, 0, mirror_flags);
>  VIR_FREE(diskAlias);
>  VIR_FREE(nbd_dest);
>  
> 

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

Re: [libvirt] [libxl] shutdown a domain before it finishes starting

[Jim Fehlig]

Zhangbo (Oscar) wrote:
> Hi all:
> Suppose we have a guest domain which is pvops, for example, rhel6.4. 
> 
> Steps to produce the problem:
> 1 start the guest by virDomainCreate() 
> 2 the API returns before the guest domain fully available, which means, 
> the disks, network interfaces and some import services are not available 
> inside the guest.
> 3 we call virDomainShutdown() to shutdown the guest.
> 
> Expected result:
>   The guest got shutdown.
> 
> The result in fact:
>   Because the guest is not available when we call virDomainShutdown(), it 
> couldn't  respond to our 'shutdown' xenstore request, the guest turns on 
> later, rather than shutting down.

I don't think this is unique to a pvops guest kernel, or even a xen stack. I see
the same behavior with qemu. 'virsh create dom.xml && virsh shutdown dom'
results in the guest kernel missing the shutdown event and booting anyhow. I
guess SeaBIOS could still be loading when the shutdown event is issued :-). The
virDomainShutdownFlags documentation even states "that the guest OS may ignore
the request". In my example, the guest OS isn't even alive yet.

> So , the question is:
>In libxl_driver( xen-hypervisor environment), how can we tell that the 
> guest is available or not, and is it suitable to shutdown the guest at that 
> moment?

libxl has no API to determine if a guest OS has booted. In a qemu/kvm stack, I
suppose qemu-ga is the preferred way to know when a guest OS has booted, or is
far enough along to respond to shutdown events.

One possible approach in xen, which is not supported by libvirt, would be to
monitor the state of a device frontend in xenstore. E.g. when
/local/domain//device/vif//state reaches 4 (connected), you'll at
least know the driver in the guest is up and running.

Regards,
Jim

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

Re: [libvirt] [RFC 6/6] qemu: Cache GIC capabilities

[John Ferlan]

On 03/21/2016 01:28 PM, Andrea Bolognani wrote:
> Implement support for saving GIC capabilities in the cache and
> read them back.
> ---
>  src/qemu/qemu_capabilities.c | 87 
> 
>  1 file changed, 87 insertions(+)
> 

This is more related to patch 2 and I think would need to be in place
for patch 5 to work properly given the right circumstances...

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 64007f0..23c3740 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2906,6 +2906,77 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const 
> char *filename,
>  }
>  VIR_FREE(nodes);
>  
> +if ((n = virXPathNodeSet("./gic", ctxt, )) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("failed to parse qemu capabilities gic"));
> +goto cleanup;
> +}
> +if (n > 0) {
> +unsigned int uintValue;
> +bool boolValue;
> +
> +qemuCaps->ngicCapabilities = n;
> +if (VIR_ALLOC_N(qemuCaps->gicCapabilities, n) < 0)
> +goto cleanup;
> +
> +for (i = 0; i < n; i++) {
> +virGICCapabilityPtr cap = >gicCapabilities[i];
> +
> +if (!(str = virXMLPropString(nodes[i], "version"))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("missing GIC version "
> + "in QEMU capabilities cache"));
> +goto cleanup;
> +}
> +if (str &&
> +virStrToLong_ui(str, NULL, 10, ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("malformed GIC version "
> + "in QEMU capabilities cache"));
> +goto cleanup;
> +}
> +cap->version = uintValue;
> +VIR_FREE(str);
> +
> +if (!(str = virXMLPropString(nodes[i], "kernel"))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("missing in-kernel GIC information "
> + "in QEMU capabilities cache"));
> +goto cleanup;
> +}
> +if (str &&
> +!(boolValue = STREQ(str, "true")) &&
> +STRNEQ(str, "false")) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("malformed in-kernel GIC information "
> + "in QEMU capabilities cache"));
> +goto cleanup;
> +}
> +if (boolValue)
> +cap->implementation |= VIR_GIC_IMPLEMENTATION_KERNEL;
> +VIR_FREE(str);
> +
> +if (!(str = virXMLPropString(nodes[i], "emulated"))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("missing emulated GIC information "
> + "in QEMU capabilities cache"));
> +goto cleanup;
> +}
> +if (str &&
> +!(boolValue = STREQ(str, "true")) &&
> +STRNEQ(str, "false")) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("malformed emulated GIC information "
> + "in QEMU capabilities cache"));
> +goto cleanup;
> +}
> +if (boolValue)
> +cap->implementation |= VIR_GIC_IMPLEMENTATION_EMULATED;


Since these are processed as either/or in patch 5 based on virttype, I'm
beginning to think perhaps you'd have :



but this works, I'm still trying to process the whole domaincaps code
and what the use case would be. It looks like merely the output to
domcapabilities. In which case, I'm wonder if printing the gic_version
inside  would be sufficient.

John
> +VIR_FREE(str);
> +}
> +}
> +VIR_FREE(nodes);
> +
>  ret = 0;
>   cleanup:
>  VIR_FREE(str);
> @@ -2972,6 +3043,22 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const 
> char *filename)
>qemuCaps->machineMaxCpus[i]);
>  }
>  
> +for (i = 0; i < qemuCaps->ngicCapabilities; i++) {
> +virGICCapabilityPtr cap;
> +bool kernel;
> +bool emulated;
> +
> +cap = >gicCapabilities[i];
> +kernel = (cap->implementation & VIR_GIC_IMPLEMENTATION_KERNEL);
> +emulated = (cap->implementation & VIR_GIC_IMPLEMENTATION_EMULATED);
> +
> +virBufferAsprintf(,
> +  "\n",
> +  cap->version,
> +  kernel ? "true" : "false",
> +  emulated ? "true" : "false");
> +}
> +
>  virBufferAdjustIndent(, -2);
>  virBufferAddLit(, "\n");
>  
> 

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

Re: [libvirt] [RFC 5/6] qemu: Fill in GIC capabilities

[John Ferlan]

On 03/21/2016 01:28 PM, Andrea Bolognani wrote:
> Take the GIC capabilities stored in a virQEMUCaps instance and
> update a virDomainCaps instance appropriately.
> ---
>  src/qemu/qemu_capabilities.c | 38 +-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 

I think domaincapstest.c should be modified to add a (new) 2.6 version
of the *.caps file. One that has the supported='yes' set.

and this is probably where the docs get modified to add the new elements...

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index e54208a..64007f0 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -4065,6 +4065,41 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr 
> qemuCaps,
>  }
>  
>  
> +static int
> +virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps,
> +virDomainCapsPtr domCaps)
> +{
> +virDomainCapsFeatureGICPtr gic = >gic;
> +size_t i;
> +
> +if (domCaps->arch != VIR_ARCH_ARMV7L &&
> +domCaps->arch != VIR_ARCH_AARCH64)
> +return 0;
> +
> +if (STRNEQ(domCaps->machine, "virt") &&
> +!STRPREFIX(domCaps->machine, "virt-"))
> +return 0;
> +
> +for (i = 0; i < qemuCaps->ngicCapabilities; i++) {
> +virGICCapabilityPtr cap = >gicCapabilities[i];
> +
> +if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM &&
> +!(cap->implementation & VIR_GIC_IMPLEMENTATION_KERNEL))
> +continue;
> +
> +if (domCaps->virttype == VIR_DOMAIN_VIRT_QEMU &&
> +!(cap->implementation & VIR_GIC_IMPLEMENTATION_EMULATED))
> +continue;

For these, patch 6 would need to be already in place I think if
circumstances were "just right" (so to speak).

> +
> +gic->supported = true;
> +VIR_DOMAIN_CAPS_ENUM_SET(gic->version,
> + cap->version);

Can there be more than one?  How is that handled!

IOW: Once we print, do we just break;?


John

> +}
> +
> +return 0;
> +}
> +
> +
>  int
>  virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
>virQEMUCapsPtr qemuCaps,
> @@ -4081,7 +4116,8 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
>  if (virQEMUCapsFillDomainOSCaps(qemuCaps, os,
>  loader, nloader) < 0 ||
>  virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, domCaps->machine, 
> disk) < 0 ||
> -virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0)
> +virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 ||
> +virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0)
>  return -1;
>  return 0;
>  }
> 

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

Re: [libvirt] [RFC 4/6] conf: Expose GIC capabilities

[John Ferlan]

On 03/21/2016 01:28 PM, Andrea Bolognani wrote:
> Add information about GIC capabilities to virDomainCaps and update
> the formatter to include them in the XML output.
> ---
>  src/conf/domain_capabilities.c   | 20 
> 
>  src/conf/domain_capabilities.h   | 10 ++
>  tests/domaincapsschemadata/domaincaps-basic.xml  |  3 +++
>  tests/domaincapsschemadata/domaincaps-full.xml   |  3 +++
>  .../domaincaps-qemu_1.6.50-1.xml |  3 +++
>  5 files changed, 39 insertions(+)
> 
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index 466c0c6..d29ed81 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -262,6 +262,18 @@ virDomainCapsDeviceHostdevFormat(virBufferPtr buf,
>  }
>  
>  
> +static void
> +virDomainCapsFeatureGICFormat(virBufferPtr buf,
> +  virDomainCapsFeatureGICPtr const gic)
> +{
> +FORMAT_PROLOGUE(gic);
> +
> +ENUM_PROCESS(gic, version, virGICVersionTypeToString);
> +
> +FORMAT_EPILOGUE(gic);
> +}
> +
> +
>  static int
>  virDomainCapsFormatInternal(virBufferPtr buf,
>  virDomainCapsPtr const caps)
> @@ -291,6 +303,14 @@ virDomainCapsFormatInternal(virBufferPtr buf,
>  virBufferAdjustIndent(buf, -2);
>  virBufferAddLit(buf, "\n");
>  
> +virBufferAddLit(buf, "\n");
> +virBufferAdjustIndent(buf, 2);
> +
> +virDomainCapsFeatureGICFormat(buf, >gic);
> +
> +virBufferAdjustIndent(buf, -2);
> +virBufferAddLit(buf, "\n");
> +
>  virBufferAdjustIndent(buf, -2);
>  virBufferAddLit(buf, "\n");
>  return 0;
> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index 3eacb35..95afe5e 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -81,6 +81,13 @@ struct _virDomainCapsDeviceHostdev {
>  /* add new fields here */
>  };
>  
> +typedef struct _virDomainCapsFeatureGIC virDomainCapsFeatureGIC;
> +typedef virDomainCapsFeatureGIC *virDomainCapsFeatureGICPtr;
> +struct _virDomainCapsFeatureGIC {
> +bool supported;

Hmmm.. I see - I guess depends on how patch 1/6 is handled...

> +virDomainCapsEnum version; /* Info about virGICVersion */
> +};
> +
>  struct _virDomainCaps {
>  virObjectLockable parent;
>  
> @@ -96,6 +103,9 @@ struct _virDomainCaps {
>  virDomainCapsDeviceDisk disk;
>  virDomainCapsDeviceHostdev hostdev;
>  /* add new domain devices here */
> +
> +virDomainCapsFeatureGIC gic;
> +/* add new domain features here */
>  };
>  
>  virDomainCapsPtr virDomainCapsNew(const char *path,
> diff --git a/tests/domaincapsschemadata/domaincaps-basic.xml 
> b/tests/domaincapsschemadata/domaincaps-basic.xml
> index 6171393..6587d56 100644
> --- a/tests/domaincapsschemadata/domaincaps-basic.xml
> +++ b/tests/domaincapsschemadata/domaincaps-basic.xml
> @@ -8,4 +8,7 @@
>  
>  
>
> +  
> +
> +  
>  
> diff --git a/tests/domaincapsschemadata/domaincaps-full.xml 
> b/tests/domaincapsschemadata/domaincaps-full.xml
> index 96202bc..d4f91fa 100644
> --- a/tests/domaincapsschemadata/domaincaps-full.xml
> +++ b/tests/domaincapsschemadata/domaincaps-full.xml
> @@ -68,4 +68,7 @@
>
>  
>
> +  
> +
> +  
>  
> diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml 
> b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
> index 37d2102..990661b 100644
> --- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
> +++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
> @@ -56,4 +56,7 @@
>
>  
>
> +  
> +
> +  
>  
> 


I first wondered why the XML file was modified, but later understood -
sort of... Still not fully clear on how what this schema output should
look like... For prior to 2.6 should it even be printed?  For 2.6 and
beyond what would it look like?

John

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

Re: [libvirt] [RFC 3/6] schema: Validate GIC capabilities

[John Ferlan]

On 03/21/2016 01:28 PM, Andrea Bolognani wrote:
> We need to expose GIC capabilities in the domain capabilities
> XML: update the schema to validate documents that contain the
> new information.
> ---
>  docs/schemas/domaincaps.rng | 18 ++
>  1 file changed, 18 insertions(+)
> 

I think patches 3 and 4 should be combined and 5 is not too far behind!

"features" is fairly generic for something so specific, but I'm not sure
I have other great suggestions for a "domain capabilities" section that
has path, domain, machine, arch, vcpu, os, and devices already. It is a
cpu interrupt controller - so maybe it's an "interrupt" device.

When the XML is created what is it going to look like?

 
   
 
   2
   3
 
   
 

Is putting it after "" a back compat thing? I guess I would
think it was more logical after the  or even more radical as part
of it:

  armv7l

Additionally, docs/formatdomaincaps.html.in will need an update to
describe this...

And then there's virsh.pod - not sure if it needs an update...

John

> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
> index 35d3745..0d2777b 100644
> --- a/docs/schemas/domaincaps.rng
> +++ b/docs/schemas/domaincaps.rng
> @@ -31,6 +31,9 @@
>  
>
>  
> +
> +  
> +
>
>  
>
> @@ -88,6 +91,21 @@
>  
>
>  
> +  
> +
> +  
> +
> +  
> +
> +  
> +
> +  
> +
> +  
> +  
> +
> +  
> +
>
>  
>
> 

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

Re: [libvirt] Host device assignment driver name vfio/ kvm

[Laine Stump]

On 03/29/2016 07:45 AM, Moshe Levi wrote:


Hi,

I was  testing Host device assignment  in OpenStack environment where 
the driver name is vfio or kvm.


My setup is as follow:

1.Fedora 21

2.Libvirt 1.3.0 which I compiled

3.OpenStack master

I have also other setups with older Libvirt version and the same 
OpenStack environment.


I notice that on my fedora environment the driver name is vfio were in 
my old environment the driver name is kvm.


According to Libvirt documentation default is "vfio" on systems where 
the VFIO driver is available and loaded, see [1]


I remove the vfio modules by removing vfio, vfio_iommu_type1, vfio_pci 
but when I boot a vm the drive name is vfio


How can change the driver name to be kvm?



libvirt tries very hard to use vfio rather than legacy kvm, because 
legacy kvm is old, deprecated, and "declared bad" :-). But it won't 
changed it to vfio if you've explicitly said that you want to use kvm. 
If you really want to use legacy kvm device assignment, manually set 
that in the config. When you do that, if the system you're running on 
doesn't support it, it will error out rather than switching.


Another thing that I encounter is an error when suspending VM (in 
OpenStack environment)  when the driver name is  vfio.


In such case I am getting  the following error from Libvirt:

2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher   
File "/usr/lib64/python2.7/site-packages/libvirt.py", line 560, in 
attachDeviceFlags


2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher 
if ret == -1: raise libvirtError ('virDomainAttachDeviceFlags() 
failed', dom=self)


2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher 
libvirtError: internal error: unable to execute QEMU command 
'device_add': Device initialization failed.


I would appreciate for some pointers on what can cause this issue.



Assuming that openstack uses libvirt's virDomainSave API I would expect 
suspending a guest to fail if it had an assigned device (since libvirt 
implements this by "migrating to disk", and qemu doesn't allow migration 
of a guest with an assigned device. But your problem is that it's trying 
to *attach* a device, which I wouldn't consider to be a part of a save 
or suspend or whatever operation. Is it possible to get more information 
about what leads up to this?



[1] https://libvirt.org/formatdomain.html#elementsHostDev 





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


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

Re: [libvirt] [RFC 2/6] qemu: Probe GIC capabilities

[John Ferlan]

On 03/21/2016 01:28 PM, Andrea Bolognani wrote:
> Use the query-gic-capabilities QMP command to probe GIC
> capabilities for a QEMU binary.
> 
> The information obtained is stored in virQEMUCaps.

Once finally pushed to the qemu.git repo, perhaps reference the qemu
commit id...

> ---
>  src/qemu/qemu_capabilities.c | 24 
>  src/qemu/qemu_monitor.c  | 10 +
>  src/qemu/qemu_monitor.h  |  4 ++
>  src/qemu/qemu_monitor_json.c | 90 
> 
>  src/qemu/qemu_monitor_json.h |  4 ++
>  src/util/virgic.h| 13 +++
>  6 files changed, 145 insertions(+)

Is there a way to set a capabilities bit to determine if the
"query-gic-capabilities" command exists? Perhaps virQEMUCapsCommands?
Changes a check later on in qemu_monitor_json.c

> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 4b1e750..e54208a 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -354,6 +354,9 @@ struct _virQEMUCaps {
>  char **machineTypes;
>  char **machineAliases;
>  unsigned int *machineMaxCpus;
> +
> +size_t ngicCapabilities;
> +virGICCapability *gicCapabilities;

Any reason to not use virGICCapabilityPtr ?

>  };
>  
>  struct virQEMUCapsSearchData {
> @@ -2690,6 +2693,22 @@ 
> virQEMUCapsProbeQMPMigrationCapabilities(virQEMUCapsPtr qemuCaps,
>  return 0;
>  }
>  

May as well go with the 2 empty lines between functions here.
Need some intro comments, arguments, returns, etc. Not every command
does it, but let's try to add some

> +static int
> +virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps,
> +   qemuMonitorPtr mon)
> +{
> +virGICCapability *caps = NULL;

virGICCapabilityPtr ?

> +int ncaps;
> +
> +if ((ncaps = qemuMonitorGetGICCapabilities(mon, )) < 0)
> +return -1;
> +
> +qemuCaps->gicCapabilities = caps;
> +qemuCaps->ngicCapabilities = ncaps;

You will need to VIR_FREE(qemuCaps->gicCapabilities) in
virQEMUCapsDispose and virQEMUCapsReset  (looked up where machineTypes
and machineAliases free'd memory).

> +
> +return 0;
> +}
> +
>  int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
>  qemuMonitorPtr mon)
>  {
> @@ -3410,6 +3429,11 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>  if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) < 0)
>  goto cleanup;
>  
> +/* GIC capabilities, eg. available GIC versions */
> +if (ARCH_IS_ARM(qemuCaps->arch) &&

I note that patch 5 only fills in the domain feature caps based on
VIR_ARCH_ARMV7L and VIR_ARCH_AARCH64...  Does that perhaps mean we need
some sort of "second" ARCH_IS_ARM type macro?  There are some other
places where the two are checked...

> +virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0)
> +goto cleanup;
> +
>  ret = 0;
>   cleanup:
>  return ret;
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index ace3bb4..ac5fe8c 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3552,6 +3552,16 @@ qemuMonitorSetMigrationCapability(qemuMonitorPtr mon,
>  
>  

Although not always done, would be nice to have some intro comments,
arguments, returns, etc.  Especially since this returns 'n' capabilities
that were found and **capabilities is allocated and must be free'd by
caller.

>  int
> +qemuMonitorGetGICCapabilities(qemuMonitorPtr mon,
> +  virGICCapability **capabilities)

virGICCapabilityPtr ?

> +{
> +QEMU_CHECK_MONITOR_JSON(mon);
> +
> +return qemuMonitorJSONGetGICCapabilities(mon, capabilities);
> +}
> +
> +
> +int
>  qemuMonitorNBDServerStart(qemuMonitorPtr mon,
>const char *host,
>unsigned int port)
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 4467a41..550eae0 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -34,6 +34,7 @@
>  # include "virnetdev.h"
>  # include "device_conf.h"
>  # include "cpu/cpu.h"
> +# include "util/virgic.h"
>  
>  typedef struct _qemuMonitor qemuMonitor;
>  typedef qemuMonitor *qemuMonitorPtr;
> @@ -545,6 +546,9 @@ int qemuMonitorSetMigrationCapability(qemuMonitorPtr mon,
>qemuMonitorMigrationCaps capability,
>bool state);
>  
> +int qemuMonitorGetGICCapabilities(qemuMonitorPtr mon,
> +  virGICCapability **capabilities);

virGICCapabilityPtr ?

> +
>  typedef enum {
>QEMU_MONITOR_MIGRATE_BACKGROUND= 1 << 0,
>QEMU_MONITOR_MIGRATE_NON_SHARED_DISK  = 1 << 1, /* migration with 
> non-shared storage with full disk copy */
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 8352e53..dee2ee4 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -5744,6 +5744,96 @@ 

Re: [libvirt] [RFC 1/6] conf: Get rid of virDomainCapsDevice

[John Ferlan]

On 03/21/2016 01:28 PM, Andrea Bolognani wrote:
> The struct contains a single boolean field which can be
> applied to domain capabilities that do not represent device
> availability.
> 
> Instead of trying to come up with a more generic name just
> get rid of the struct altogether.
> ---
>  src/conf/domain_capabilities.c |  6 +++---
>  src/conf/domain_capabilities.h | 14 --
>  src/qemu/qemu_capabilities.c   |  8 
>  tests/domaincapstest.c |  8 
>  4 files changed, 15 insertions(+), 21 deletions(-)
> 

The construct was added as part of commit id '614581f32' - not sure if
Michal felt more bool's were going to be added to virDomainCapsDevice.

I see this affects patch 4 - I think it would be a good idea to see if
Michal had "other designs" in mind before changing this. That could be
separate too...


John
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index 0e32f52..466c0c6 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -187,9 +187,9 @@ virDomainCapsStringValuesFormat(virBufferPtr buf,
>  #define FORMAT_PROLOGUE(item)   \
>  do {\
>  virBufferAsprintf(buf, "<" #item " supported='%s'%s\n", \
> -  item->device.supported ? "yes" : "no",\
> -  item->device.supported ? ">" : "/>"); \
> -if (!item->device.supported)\
> +  item->supported ? "yes" : "no",   \
> +  item->supported ? ">" : "/>");\
> +if (!item->supported)   \
>  return; \
>  virBufferAdjustIndent(buf, 2);  \
>  } while (0)
> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index 597ac75..3eacb35 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -44,16 +44,10 @@ struct _virDomainCapsStringValues {
>  size_t nvalues; /* number of strings */
>  };
>  
> -typedef struct _virDomainCapsDevice virDomainCapsDevice;
> -typedef virDomainCapsDevice *virDomainCapsDevicePtr;
> -struct _virDomainCapsDevice {
> -bool supported; /* true if  is supported by hypervisor */
> -};
> -
>  typedef struct _virDomainCapsLoader virDomainCapsLoader;
>  typedef virDomainCapsLoader *virDomainCapsLoaderPtr;
>  struct _virDomainCapsLoader {
> -virDomainCapsDevice device;
> +bool supported;
>  virDomainCapsStringValues values;   /* Info about values for the element 
> */
>  virDomainCapsEnum type; /* Info about virDomainLoader */
>  virDomainCapsEnum readonly; /* Info about readonly:virTristateBool */
> @@ -62,14 +56,14 @@ struct _virDomainCapsLoader {
>  typedef struct _virDomainCapsOS virDomainCapsOS;
>  typedef virDomainCapsOS *virDomainCapsOSPtr;
>  struct _virDomainCapsOS {
> -virDomainCapsDevice device;
> +bool supported;
>  virDomainCapsLoader loader; /* Info about virDomainLoaderDef */
>  };
>  
>  typedef struct _virDomainCapsDeviceDisk virDomainCapsDeviceDisk;
>  typedef virDomainCapsDeviceDisk *virDomainCapsDeviceDiskPtr;
>  struct _virDomainCapsDeviceDisk {
> -virDomainCapsDevice device;
> +bool supported;
>  virDomainCapsEnum diskDevice;   /* Info about virDomainDiskDevice enum 
> values */
>  virDomainCapsEnum bus;  /* Info about virDomainDiskBus enum 
> values */
>  /* add new fields here */
> @@ -78,7 +72,7 @@ struct _virDomainCapsDeviceDisk {
>  typedef struct _virDomainCapsDeviceHostdev virDomainCapsDeviceHostdev;
>  typedef virDomainCapsDeviceHostdev *virDomainCapsDeviceHostdevPtr;
>  struct _virDomainCapsDeviceHostdev {
> -virDomainCapsDevice device;
> +bool supported;
>  virDomainCapsEnum mode; /* Info about virDomainHostdevMode */
>  virDomainCapsEnum startupPolicy;/* Info about virDomainStartupPolicy 
> */
>  virDomainCapsEnum subsysType;   /* Info about 
> virDomainHostdevSubsysType */
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index b223837..4b1e750 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -3909,7 +3909,7 @@ virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps,
>  {
>  size_t i;
>  
> -capsLoader->device.supported = true;
> +capsLoader->supported = true;
>  
>  if (VIR_ALLOC_N(capsLoader->values.values, nloader) < 0)
>  return -1;
> @@ -3951,7 +3951,7 @@ virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps,
>  {
>  virDomainCapsLoaderPtr capsLoader = >loader;
>  
> -os->device.supported = true;
> +os->supported = true;
>  if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, capsLoader,
>  loader, 

Re: [libvirt] [PATCH 00/15] support for pxb and pxb-pcie controllers

[Michal Privoznik]

On 30.03.2016 16:48, Laine Stump wrote:
> On 03/30/2016 05:05 AM, Michal Privoznik wrote:
>> On 24.03.2016 20:25, Laine Stump wrote:
>>> These two controllers are used to create a new root bus on a 440fx
>>> (pxb) or q35 (pxb-pie) virtual machine. There may be other use cases,
>>> but the main reason for me taking the time to support a separate root
>>> bus is to have assigned devices be visible in the guest on a different
>>> NUMA node, so that the guest can be aware of the locality of the
>>> device wrt CPU and memory that are on different NUMA nodes - although
>>> you aren't required to, you can add a N subelement to the
>>> bus'  element to indicate which NUMA node it is on (it's up to
>>> the management application to place devices on that bus that really
>>> are on the given NUMA node in the host).
>>>
>>> There are several differences between pxb and pxb-pcie, which are
>>> detailed in the individual commit log messages, but in short:
>>>
>>> 1) pxb is for 440fx, pxb-pcie for q35 (they *might* work on other
>>> arches/machinetypes that have a PCI or PCIe bus, but I haven't enabled
>>> that)
>>>
>>> 2) pxb has an integrate d pci-bridge with 32 slots that are (should
>>> be) hotplug-capable, while pxb-pcie supplies only a single slot, and
>>> it will only accept a pcie-root-port (which will then accept a single
>>> device, hotplug-capable) or a pcie-switch-upstream-port.
>>>
>>> Along the way I encountered a few minor problems/ugliness that I took
>>> care of in patches 01/15 - 09/15. pxb support is in 10-12, and
>>> pxb-pcie is in 13-15
>>>
>>> There is a bugzilla record associated with this:
>>>
>>>https://bugzilla.redhat.com/show_bug.cgi?id=1103314
>>>
>>> Laine Stump (15):
>>>schema: make pci slot and function optional
>>>schema: rename uint8range/uint24range to uint8/uint24
>>>schema: new basic type - uint16
>>>schema: allow pci address attributes to be in decimal
>>>conf: use #define instead of literal for highest slot in upstream
>>> port
>>>conf: allow use of slot 0 in a dmi-to-pci-bridge
>>>conf/qemu: change the way VIR_PCI_CONNECT_TYPE_* flags work
>>>conf: utility function to convert PCI controller model into connect
>>>  type
>>>qemu: set PCI controller default modelName in a separate function
>>>qemu: add capabilities bit for device "pxb"
>>>conf: new pci controller model pci-expander-bus
>>>qemu: support new pci controller model "pci-expander-bus"
>>>qemu: add capabilities bit for device "pxb-pcie"
>>>conf: new pci controller model pcie-expander-bus
>>>qemu: support new pci controller model "pcie-expander-bus"
>>>
>>>   docs/formatdomain.html.in  |  74 +++-
>>>   docs/schemas/basictypes.rng|  63 ++--
>>>   docs/schemas/domaincommon.rng  |  23 +-
>>>   docs/schemas/networkcommon.rng |  12 +-
>>>   docs/schemas/nwfilter.rng  |  16 +-
>>>   src/bhyve/bhyve_device.c   |  10 +-
>>>   src/conf/domain_addr.c | 119 +--
>>>   src/conf/domain_addr.h |  68 ++--
>>>   src/conf/domain_conf.c |  61 +++-
>>>   src/conf/domain_conf.h |  11 +-
>>>   src/libvirt_private.syms   |   1 +
>>>   src/qemu/qemu_capabilities.c   |   4 +
>>>   src/qemu/qemu_capabilities.h   |   2 +
>>>   src/qemu/qemu_command.c|  74 
>>>   src/qemu/qemu_domain.c |  42 +++
>>>   src/qemu/qemu_domain_address.c | 305
>>> +---
>>>   tests/qemucapabilitiesdata/caps_2.4.0-1.caps   |   1 +
>>>   tests/qemucapabilitiesdata/caps_2.5.0-1.caps   |   1 +
>>>   tests/qemucapabilitiesdata/caps_2.6.0-1.caps   |   2 +
>>>   tests/qemucapabilitiesdata/caps_2.6.0-1.replies|   3 +
>>>   .../qemuxml2argv-aarch64-virtio-pci-default.args   |   2 +-
>>>   ...l2argv-aarch64-virtio-pci-manual-addresses.args |   2 +-
>>>   .../qemuxml2argv-pci-expander-bus-bad-machine.xml  | 167 +
>>>   .../qemuxml2argv-pci-expander-bus-bad-node.xml | 160 +
>>>   .../qemuxml2argv-pci-expander-bus.args |  87 +
>>>   .../qemuxml2argv-pci-expander-bus.xml  | 167 +
>>>   .../qemuxml2argv-pcie-expander-bus-bad-machine.xml |  36 ++
>>>   .../qemuxml2argv-pcie-expander-bus.args| 123 +++
>>>   .../qemuxml2argv-pcie-expander-bus.xml | 247 +
>>>   .../qemuxml2argv-pcie-root-port.args   |   2 +-
>>>   tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args |   2 +-
>>>   .../qemuxml2argv-pcie-switch-downstream-port.args  |   2 +-
>>>   .../qemuxml2argv-pcie-switch-upstream-port.args|   2 +-
>>>   .../qemuxml2argv-pcihole64-q35.args|   2 +-
>>>   

Re: [libvirt] [PATCH] host-validate: Improve CPU flags processing

[Andrea Bolognani]

On Wed, 2016-03-30 at 09:28 +0200, Andrea Bolognani wrote:
> On Wed, 2016-03-30 at 08:36 +0200, Peter Krempa wrote:
> > 
> > Since you are already using libvirt's utils ...
> [...]
> > 
> > Why not virStringSplit ...
> [...]
> > 
> > And STREQ?
> 
> No reason at all, really :)

On second thought, after having actually tried to implement
your suggestions...

virStringSplit() only supports a single delimiter, while
strtok_r() supports an arbitrary number - I'm using three in
my case.

So AFAICS the options are

  - call virStringSplit() multiple times, hoping to take
all possible combinations into account

  - enhance virStringSplit() to support multiple delimiters

  - keep using strtok_r()

I'm quite partial to the last option :P
What about you?

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH 0/3] qemu: fix automatic NUMA pinning with memory hotplug

[Peter Krempa]

On Wed, Mar 30, 2016 at 07:26:09 -0400, John Ferlan wrote:
> 
> 
> On 03/29/2016 10:52 AM, Peter Krempa wrote:
> > We forgot to propagate the automatic nodeset and reported non-ideal errors.
> > 
> > Peter Krempa (3):
> >   qemu: command: Pass numad nodeset when formatting memory devices at
> > boot
> >   qemu: command: Split up formatting of -numa and memory devices
> >   qemu: hotplug: Improve error when hotpluging memory with
> > auto-placement
> > 
> >  src/conf/numa_conf.c | 20 +---
> >  src/conf/numa_conf.h |  1 +
> >  src/libvirt_private.syms |  1 +
> >  src/qemu/qemu_command.c  | 27 ++-
> >  src/qemu/qemu_hotplug.c  |  7 +++
> >  5 files changed, 40 insertions(+), 16 deletions(-)
> > 
> 
> ACK series (seems safe to me),

I'll shortly push patches 1 and 2 since 1 is a pretty straightforward
bug fix and 2 is a no-op. In patch 3 I've noticed that the
function I've created would be duplicate with
virDomainNumatuneHasPlacementAuto so I'll repost it later.

Thanks.

Peter


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

[libvirt] [PATCH 2/3] qemu: Move channel path generation out of command creation

[Martin Kletzander]

Put it into separate function called qemuDomainPrepareChannel() and call
it from the new qemuProcessPrepareDomain().

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_command.c | 25 +++--
 src/qemu/qemu_command.h |  5 ++---
 src/qemu/qemu_domain.c  | 19 +++
 src/qemu/qemu_domain.h  |  4 
 src/qemu/qemu_process.c | 12 
 5 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 45c5398ad8b9..bb9357e41c3b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8468,8 +8468,7 @@ static int
 qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
  virCommandPtr cmd,
  const virDomainDef *def,
- virQEMUCapsPtr qemuCaps,
- const char *domainChannelTargetDir)
+ virQEMUCapsPtr qemuCaps)
 {
 size_t i;

@@ -8508,22 +8507,6 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
 return -1;
 }

-/*
- * TODO: Refactor so that we generate this (and onther
- * things) somewhere else then where we are building the
- * command line.
- */
-if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
-!channel->source.data.nix.path) {
-if (virAsprintf(>source.data.nix.path,
-"%s/%s", domainChannelTargetDir,
-channel->target.name ? channel->target.name
-: "unknown.sock") < 0)
-return -1;
-
-channel->source.data.nix.listen = true;
-}
-
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) &&
 channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) {
 /* spicevmc was originally introduced via a -device
@@ -9198,8 +9181,7 @@ qemuBuildCommandLine(virConnectPtr conn,
  virBitmapPtr nodeset,
  size_t *nnicindexes,
  int **nicindexes,
- const char *domainLibDir,
- const char *domainChannelTargetDir)
+ const char *domainLibDir)
 {
 size_t i;
 char uuid[VIR_UUID_STRING_BUFLEN];
@@ -9340,8 +9322,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 if (qemuBuildParallelsCommandLine(logManager, cmd, def, qemuCaps) < 0)
 goto error;

-if (qemuBuildChannelsCommandLine(logManager, cmd, def, qemuCaps,
- domainChannelTargetDir) < 0)
+if (qemuBuildChannelsCommandLine(logManager, cmd, def, qemuCaps) < 0)
 goto error;

 if (qemuBuildConsoleCommandLine(logManager, cmd, def, qemuCaps) < 0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 7c13d459f8c5..60dc76c45bdb 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -74,10 +74,9 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
virBitmapPtr nodeset,
size_t *nnicindexes,
int **nicindexes,
-   const char *domainLibDir,
-   const char *domainChannelTargetDir)
+   const char *domainLibDir)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11)
-ATTRIBUTE_NONNULL(17) ATTRIBUTE_NONNULL(18);
+ATTRIBUTE_NONNULL(17);

 /* Generate '-device' string for chardev device */
 int
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 403f01e75e46..645e1232d2f9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4639,3 +4639,22 @@ qemuDomainDiskByName(virDomainDefPtr def,

 return ret;
 }
+
+int
+qemuDomainPrepareChannel(virDomainChrDefPtr channel,
+ const char *domainChannelTargetDir)
+{
+if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
+channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
+!channel->source.data.nix.path) {
+if (virAsprintf(>source.data.nix.path,
+"%s/%s", domainChannelTargetDir,
+channel->target.name ? channel->target.name
+: "unknown.sock") < 0)
+return -1;
+
+channel->source.data.nix.listen = true;
+}
+
+return 0;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 02c6012c9c87..e893aa17cacd 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -560,4 +560,8 @@ int qemuDomainSetPrivatePaths(char **domainLibDir,

 virDomainDiskDefPtr qemuDomainDiskByName(virDomainDefPtr def, const char 
*name);

+int
+qemuDomainPrepareChannel(virDomainChrDefPtr chr,
+ const char *domainChannelTargetDir);
+
 

[libvirt] [PATCH 3/3] qemu: Generate channel target paths on hotplug as well

[Martin Kletzander]

Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9, qemu agent
channel cannot be plugged in because we won't generate its path
automatically.  Let's not only fix that, but also add tests for it so
next time it's checked for.

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

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_hotplug.c|  3 ++
 tests/qemuhotplugtest.c| 15 ++
 .../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58 ++
 .../qemuhotplug-hotplug-base+qemu-agent.xml| 58 ++
 .../qemuhotplug-qemu-agent-detach.xml  |  5 ++
 .../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml |  5 ++
 6 files changed, 144 insertions(+)
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e82dbf5448dc..b7741e15d445 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1565,6 +1565,9 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 goto cleanup;
 }

+if (qemuDomainPrepareChannel(chr, priv->channelTargetDir) < 0)
+goto cleanup;
+
 if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
 goto cleanup;

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 2b0de94fb4a6..384b7b9592b9 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -98,6 +98,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,

 (*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;

+if (qemuDomainSetPrivatePaths(>libDir,
+  >channelTargetDir,
+  "/var/lib/libvirt",
+  "/var/lib/libvirt/qemu/channel/target",
+  (*vm)->def->name,
+  (*vm)->def->id) < 0)
+goto cleanup;
+
 ret = 0;
  cleanup:
 return ret;
@@ -495,6 +503,13 @@ mymain(void)
"device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK,
"human-monitor-command", HMP(""));

+DO_TEST_ATTACH_LIVE("hotplug-base", "qemu-agent", false, true,
+"chardev-add", QMP_OK,
+"device_add", QMP_OK);
+DO_TEST_DETACH("hotplug-base", "qemu-agent-detach", false, false,
+   "device_del", QMP_OK,
+   "chardev-remove", QMP_OK);
+
 qemuTestDriverFree();
 return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
 }
diff --git 
a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml 
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
new file mode 100644
index ..2c449f1d49cd
--- /dev/null
+++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
@@ -0,0 +1,58 @@
+
+  hotplug
+  d091ea82-29e6-2e34-3005-f02617b36e87
+  4194304
+  4194304
+  4
+  
+hvm
+
+  
+  
+
+
+
+  
+  
+  destroy
+  restart
+  restart
+  
+/usr/libexec/qemu-kvm
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+
+
+  
+  
+
+
+  
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+
+  
+  
+
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml 
b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
new file mode 100644
index ..2c449f1d49cd
--- /dev/null
+++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
@@ -0,0 +1,58 @@
+
+  hotplug
+  d091ea82-29e6-2e34-3005-f02617b36e87
+  4194304
+  4194304
+  4
+  
+hvm
+
+  
+  
+
+
+
+  
+  
+  destroy
+  restart
+  restart
+  
+/usr/libexec/qemu-kvm
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+
+
+  
+  
+
+
+  
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+
+  
+  
+
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml 
b/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
new file mode 100644
index ..0c3c70a78e89
--- /dev/null
+++ b/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
@@ -0,0 +1,5 @@
+
+  
+  
+  
+
diff --git a/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml 
b/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
new file mode 100644
index ..f0e90dea3cc2
--- /dev/null
+++ b/tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
@@ -0,0 +1,5 @@
+
+  
+  
+  
+
-- 
2.8.0

--

[libvirt] [PATCH 1/3] qemuhotplugtest: Allow testing of live data

[Martin Kletzander]

For now, the test was dumping an XML of inactive domain (well, setting
the id to '-1' to be precise) when checking the results.  This patch
enables future additions to test the live XML output as well.

Signed-off-by: Martin Kletzander 
---
 tests/qemuhotplugtest.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 1eb2b6a881f2..2b0de94fb4a6 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -52,6 +52,7 @@ struct qemuHotplugTestData {
 bool keep;
 virDomainObjPtr vm;
 bool deviceDeletedEvent;
+bool live;
 };

 static int
@@ -177,12 +178,13 @@ static int
 testQemuHotplugCheckResult(virDomainObjPtr vm,
const char *expected,
const char *expectedFile,
-   bool fail)
+   bool fail, bool live)
 {
 char *actual;
 int ret;

-vm->def->id = -1;
+if (!live)
+vm->def->id = -1;
 actual = virDomainDefFormat(vm->def, driver.caps,
 VIR_DOMAIN_DEF_FORMAT_SECURE);
 if (!actual)
@@ -219,6 +221,7 @@ testQemuHotplug(const void *data)
 const char *const *tmp;
 bool fail = test->fail;
 bool keep = test->keep;
+bool live = test->live;
 unsigned int device_parse_flags = 0;
 virDomainObjPtr vm = NULL;
 virDomainDeviceDefPtr dev = NULL;
@@ -300,14 +303,14 @@ testQemuHotplug(const void *data)
 }
 if (ret == 0 || fail)
 ret = testQemuHotplugCheckResult(vm, result_xml,
- result_filename, fail);
+ result_filename, fail, live);
 break;

 case DETACH:
 ret = testQemuHotplugDetach(vm, dev);
 if (ret == 0 || fail)
 ret = testQemuHotplugCheckResult(vm, domain_xml,
- domain_filename, fail);
+ domain_filename, fail, live);
 break;

 case UPDATE:
@@ -371,7 +374,7 @@ mymain(void)
 /* wait only 100ms for DEVICE_DELETED event */
 qemuDomainRemoveDeviceWaitTime = 100;

-#define DO_TEST(file, ACTION, dev, event, fial, kep, ...)   \
+#define DO_TEST(file, ACTION, dev, event, fial, kep, liv, ...)  \
 do {\
 const char *my_mon[] = { __VA_ARGS__, NULL};\
 const char *name = file " " #ACTION " " dev;\
@@ -381,25 +384,29 @@ mymain(void)
 data.fail = fial;   \
 data.mon = my_mon;  \
 data.keep = kep;\
+data.live = liv;\
 data.deviceDeletedEvent = event;\
 if (virtTestRun(name, testQemuHotplug, ) < 0)  \
 ret = -1;   \
 } while (0)

 #define DO_TEST_ATTACH(file, dev, fial, kep, ...)   \
-DO_TEST(file, ATTACH, dev, false, fial, kep, __VA_ARGS__)
+DO_TEST(file, ATTACH, dev, false, fial, kep, false, __VA_ARGS__)
+
+#define DO_TEST_ATTACH_LIVE(file, dev, fial, kep, ...)  \
+DO_TEST(file, ATTACH, dev, false, fial, kep, true, __VA_ARGS__)

 #define DO_TEST_DETACH(file, dev, fial, kep, ...)   \
-DO_TEST(file, DETACH, dev, false, fial, kep, __VA_ARGS__)
+DO_TEST(file, DETACH, dev, false, fial, kep, false, __VA_ARGS__)

 #define DO_TEST_ATTACH_EVENT(file, dev, fial, kep, ...) \
-DO_TEST(file, ATTACH, dev, true, fial, kep, __VA_ARGS__)
+DO_TEST(file, ATTACH, dev, true, fial, kep, false, __VA_ARGS__)

 #define DO_TEST_DETACH_EVENT(file, dev, fial, kep, ...) \
-DO_TEST(file, DETACH, dev, true, fial, kep, __VA_ARGS__)
+DO_TEST(file, DETACH, dev, true, fial, kep, false, __VA_ARGS__)

 #define DO_TEST_UPDATE(file, dev, fial, kep, ...)   \
-DO_TEST(file, UPDATE, dev, false, fial, kep, __VA_ARGS__)
+DO_TEST(file, UPDATE, dev, false, fial, kep, false, __VA_ARGS__)


 #define QMP_OK  "{\"return\": {}}"
-- 
2.8.0

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

[libvirt] [PATCH 0/3] qemu: Fix hotplug of guest agent

[Martin Kletzander]

Martin Kletzander (3):
  qemuhotplugtest: Allow testing of live data
  qemu: Move channel path generation out of command creation
  qemu: Generate channel target paths on hotplug as well

 src/qemu/qemu_command.c| 25 ++
 src/qemu/qemu_command.h|  5 +-
 src/qemu/qemu_domain.c | 19 +++
 src/qemu/qemu_domain.h |  4 ++
 src/qemu/qemu_hotplug.c|  3 ++
 src/qemu/qemu_process.c| 12 +++--
 tests/qemuhotplugtest.c| 42 
 .../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58 ++
 .../qemuhotplug-hotplug-base+qemu-agent.xml| 58 ++
 .../qemuhotplug-qemu-agent-detach.xml  |  5 ++
 .../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml |  5 ++
 11 files changed, 197 insertions(+), 39 deletions(-)
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml

--
2.8.0

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

Re: [libvirt] [PATCH v7 0/4] ARM: add query-gic-capabilities QMP command

[Peter Maydell]

On 24 March 2016 at 02:55, Peter Xu  wrote:
> v7 changes:
> - patch 1
>   - add more to commit log, about how to use the results [Markus]
>   - change interface from dict back to array [Markus, Eric]
> - patch 2
>   - kvm_arm_create_scratch_host_vcpu should raise error when init
> non-zero, but failed to find a good CPU model [Sergey]
> - patch 3
>   - rename function to kvm_device_supported [Sergey]
> - patch 4
>   - comment line from "FIXME" to "TODO" [Sergey]
>   - make kvm-related part another function [Sergey]
>   - change result to array to follow patch 1's change

Applied this version to target-arm.next, thanks.

-- PMM

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

Re: [libvirt] [PATCH 00/15] support for pxb and pxb-pcie controllers

[Laine Stump]

On 03/30/2016 05:05 AM, Michal Privoznik wrote:

On 24.03.2016 20:25, Laine Stump wrote:

These two controllers are used to create a new root bus on a 440fx
(pxb) or q35 (pxb-pie) virtual machine. There may be other use cases,
but the main reason for me taking the time to support a separate root
bus is to have assigned devices be visible in the guest on a different
NUMA node, so that the guest can be aware of the locality of the
device wrt CPU and memory that are on different NUMA nodes - although
you aren't required to, you can add a N subelement to the
bus'  element to indicate which NUMA node it is on (it's up to
the management application to place devices on that bus that really
are on the given NUMA node in the host).

There are several differences between pxb and pxb-pcie, which are
detailed in the individual commit log messages, but in short:

1) pxb is for 440fx, pxb-pcie for q35 (they *might* work on other
arches/machinetypes that have a PCI or PCIe bus, but I haven't enabled
that)

2) pxb has an integrate d pci-bridge with 32 slots that are (should
be) hotplug-capable, while pxb-pcie supplies only a single slot, and
it will only accept a pcie-root-port (which will then accept a single
device, hotplug-capable) or a pcie-switch-upstream-port.

Along the way I encountered a few minor problems/ugliness that I took
care of in patches 01/15 - 09/15. pxb support is in 10-12, and
pxb-pcie is in 13-15

There is a bugzilla record associated with this:

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

Laine Stump (15):
   schema: make pci slot and function optional
   schema: rename uint8range/uint24range to uint8/uint24
   schema: new basic type - uint16
   schema: allow pci address attributes to be in decimal
   conf: use #define instead of literal for highest slot in upstream port
   conf: allow use of slot 0 in a dmi-to-pci-bridge
   conf/qemu: change the way VIR_PCI_CONNECT_TYPE_* flags work
   conf: utility function to convert PCI controller model into connect
 type
   qemu: set PCI controller default modelName in a separate function
   qemu: add capabilities bit for device "pxb"
   conf: new pci controller model pci-expander-bus
   qemu: support new pci controller model "pci-expander-bus"
   qemu: add capabilities bit for device "pxb-pcie"
   conf: new pci controller model pcie-expander-bus
   qemu: support new pci controller model "pcie-expander-bus"

  docs/formatdomain.html.in  |  74 +++-
  docs/schemas/basictypes.rng|  63 ++--
  docs/schemas/domaincommon.rng  |  23 +-
  docs/schemas/networkcommon.rng |  12 +-
  docs/schemas/nwfilter.rng  |  16 +-
  src/bhyve/bhyve_device.c   |  10 +-
  src/conf/domain_addr.c | 119 +--
  src/conf/domain_addr.h |  68 ++--
  src/conf/domain_conf.c |  61 +++-
  src/conf/domain_conf.h |  11 +-
  src/libvirt_private.syms   |   1 +
  src/qemu/qemu_capabilities.c   |   4 +
  src/qemu/qemu_capabilities.h   |   2 +
  src/qemu/qemu_command.c|  74 
  src/qemu/qemu_domain.c |  42 +++
  src/qemu/qemu_domain_address.c | 305 +---
  tests/qemucapabilitiesdata/caps_2.4.0-1.caps   |   1 +
  tests/qemucapabilitiesdata/caps_2.5.0-1.caps   |   1 +
  tests/qemucapabilitiesdata/caps_2.6.0-1.caps   |   2 +
  tests/qemucapabilitiesdata/caps_2.6.0-1.replies|   3 +
  .../qemuxml2argv-aarch64-virtio-pci-default.args   |   2 +-
  ...l2argv-aarch64-virtio-pci-manual-addresses.args |   2 +-
  .../qemuxml2argv-pci-expander-bus-bad-machine.xml  | 167 +
  .../qemuxml2argv-pci-expander-bus-bad-node.xml | 160 +
  .../qemuxml2argv-pci-expander-bus.args |  87 +
  .../qemuxml2argv-pci-expander-bus.xml  | 167 +
  .../qemuxml2argv-pcie-expander-bus-bad-machine.xml |  36 ++
  .../qemuxml2argv-pcie-expander-bus.args| 123 +++
  .../qemuxml2argv-pcie-expander-bus.xml | 247 +
  .../qemuxml2argv-pcie-root-port.args   |   2 +-
  tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args |   2 +-
  .../qemuxml2argv-pcie-switch-downstream-port.args  |   2 +-
  .../qemuxml2argv-pcie-switch-upstream-port.args|   2 +-
  .../qemuxml2argv-pcihole64-q35.args|   2 +-
  .../qemuxml2argv-q35-pm-disable-fallback.args  |   2 +-
  .../qemuxml2argv-q35-pm-disable.args   |   2 +-
  .../qemuxml2argv-q35-usb2-multi.args   |   2 +-
  .../qemuxml2argv-q35-usb2-reorder.args |   2 +-
  tests/qemuxml2argvdata/qemuxml2argv-q35-usb2.args  |   2 +-
  tests/qemuxml2argvdata/qemuxml2argv-q35.args   |   2 +-
  

Re: [libvirt] [PATCH] docs: Don't use bold text in headers and menu items

[Andrea Bolognani]

On Thu, 2016-03-10 at 16:17 -0500, Cole Robinson wrote:
> On 03/10/2016 05:49 AM, Andrea Bolognani wrote:
> > 
> > Now that headers and menu items are using a bigger font and more
> > generous spacing, they don't really need to be bold to stand out.
> 
> I tried it, but I kinda like the bold... I think it helps when scrolling
> through formatdomain.html at least. But if anyone else wants to ACK I won't 
> object

I've taken another stab[1] at this by turning all the headers bold,
but reducing them in size to maintain the balance.

I think it looks even better, so thanks for your feedback :)

Cheers.


[1] https://www.redhat.com/archives/libvir-list/2016-March/msg01443.html
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

[libvirt] [PATCH 6/7] docs: Use bold text for all headers

[Andrea Bolognani]

All headers except for  were already bold: make it bold as
well to increase visual consistency.
---
 docs/libvirt.css | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/docs/libvirt.css b/docs/libvirt.css
index d7a4d95..c6f6d0d 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -1,9 +1,4 @@
-h1 {
-font-weight: normal;
-color: #3c857c;
-}
-
-h2, h3, h4, h5, h6 {
+h1, h2, h3, h4, h5, h6 {
 color: #3c857c;
 }
 
-- 
2.5.5

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

Re: [libvirt] [PATCH 04/15] schema: allow pci address attributes to be in decimal

[Laine Stump]

On 03/30/2016 05:05 AM, Michal Privoznik wrote:

On 24.03.2016 20:25, Laine Stump wrote:

This is especially useful for "bus", since the bus of a device's pci
address is matched to the "index" of a controller to determine which
bus it will be connected to, and "index" is always specified in
decimal - being able to specify both in decimal at least makes it
easier to assure a device is being assigned to the correct bus when it
is added. For the other attributes, it is just a convenience.

(MB: the parser already allows for any of these attributes to be given
in decimal, and there are even examples floating around on the
internet that give them in decimal rather than hex (written in the
days before virsh did schema validation on all XML). This only updates
the schema to match the parser.)
---
  docs/schemas/basictypes.rng | 32 
  1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 64babaf..e2936d8 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -282,24 +282,32 @@

  


-
-  (0x)?[0-9a-fA-F]{1,4}
-
+


-
-  (0x)?[0-9a-fA-F]{1,2}
-
+


-
-  (0x)?[0-1]?[0-9a-fA-F]
-
+
+  
+(0x)?[0-1]?[0-9a-fA-F]
+  
+  
+0
+31
+  
+


-
-  (0x)?[0-7]
-
+
+  
+(0x)?[0-7]
+  
+  
+0
+7
+  
+

  




Okay, but we can never format them in decimal as '12' in decimal is
something different than in hexadecimal.


Correct. I think we're stuck with formatting the way it is (actually, 
since it's so common for PCI addresses to be in hex, if we were going to 
change anything it would be to format the index in hex, but even that 
would probably confuse some people).


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

[libvirt] [PATCH 0/7] docs: Some website tweaks

[Andrea Bolognani]

Basically, the navigation menu is *way* too big, and takes
the focus away from the actual content. Plus it's very easy
for longish words to overflow it (see "Authentication" in
the screenshots below).

The headers are probably bigger than needed as well, so
I've scaled them down. Using Wikipedia for comparison,
you can see the new size is more comparable to what they
use.

Some other minor cleanups have been included as well.

Before[1] and after[2] screenshots for your convenience.

Cheers.


[1] http://i.imgur.com/MZchAWD.png
[2] http://i.imgur.com/BXepLTh.png

Andrea Bolognani (7):
  docs: Adjust vertical whitespace in CSS
  docs: Remove empty CSS rule
  docs: Don't use  in headers
  docs: Make menu entries smaller
  docs: Don't use bold text for menu entries
  docs: Use bold text for all headers
  docs: Make most headers a bit smaller

 docs/apps.html.in |  2 +-
 docs/generic.css  | 15 +--
 docs/libvirt.css  | 30 ++
 3 files changed, 16 insertions(+), 31 deletions(-)

-- 
2.5.5

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

[libvirt] [PATCH 7/7] docs: Make most headers a bit smaller

[Andrea Bolognani]

Headers are bold already, so the font doesn't need to be that
big to draw attention.
---
 docs/generic.css | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/generic.css b/docs/generic.css
index d98f612..c0c6215 100644
--- a/docs/generic.css
+++ b/docs/generic.css
@@ -52,22 +52,22 @@ h1, h2, h3, h4, h5, h6 {
 
 h1 {
   margin-top: 0em;
-  font-size: 2em;
+  font-size: 1.6em;
 }
 
 h2 {
   margin-top: 1.0em;
-  font-size: 1.6em;
+  font-size: 1.4em;
 }
 
 h3 {
   margin-top: 1.0em;
-  font-size: 1.4em;
+  font-size: 1.2em;
 }
 
 h4 {
   margin-top: 1.0em;
-  font-size: 1.2em;
+  font-size: 1.1em;
 }
 
 h5 {
-- 
2.5.5

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

[libvirt] [PATCH 3/7] docs: Don't use in headers

[Andrea Bolognani]

There's only one instance of that happening, and it looks
kinda off. Get rid of it, along with the corresponding
CSS rules.
---
 docs/apps.html.in | 2 +-
 docs/libvirt.css  | 4 
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 39fb78d..b8337b9 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -2,7 +2,7 @@
 http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd;>
 http://www.w3.org/1999/xhtml;>
   
-Applications using libvirt
+Applications using libvirt
 
 
   This page provides an illustration of the wide variety of
diff --git a/docs/libvirt.css b/docs/libvirt.css
index d560070..637ed35 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -3,10 +3,6 @@ h1 {
 color: #3c857c;
 }
 
-h1 strong {
-font-weight: bold;
-}
-
 h2, h3, h4, h5, h6 {
 color: #3c857c;
 }
-- 
2.5.5

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

[libvirt] [PATCH 2/7] docs: Remove empty CSS rule

[Andrea Bolognani]

---
 docs/libvirt.css | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/docs/libvirt.css b/docs/libvirt.css
index 4fffa18..d560070 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -292,9 +292,6 @@ p.image {
 width: 99%;
 }
 
-#projects {
-}
-
 #projects span {
 font-size: 0.8em;
 display: block;
-- 
2.5.5

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

[libvirt] [PATCH 5/7] docs: Don't use bold text for menu entries

[Andrea Bolognani]

The menu should not take the focus away from the actual contents.
---
 docs/libvirt.css | 1 -
 1 file changed, 1 deletion(-)

diff --git a/docs/libvirt.css b/docs/libvirt.css
index fdbd99e..d7a4d95 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -52,7 +52,6 @@ h2, h3, h4, h5, h6 {
 padding: 0px;
 margin: 0px;
 border: 0px;
-font-weight: bold;
 }
 
 #menu ul li a, #menu ul li span {
-- 
2.5.5

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

[libvirt] [PATCH 1/7] docs: Adjust vertical whitespace in CSS

[Andrea Bolognani]

Ensure all CSS rules are separated with a single blank line.
---
 docs/generic.css |  7 +--
 docs/libvirt.css | 12 ++--
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/docs/generic.css b/docs/generic.css
index aeea17f..d98f612 100644
--- a/docs/generic.css
+++ b/docs/generic.css
@@ -16,7 +16,6 @@ div.body p:first-letter {
   font-weight: bold;
 }
 
-
 p, ul, ol, dl {
   padding: 0px;
   margin: 0px;
@@ -43,7 +42,6 @@ dl dd {
   margin-bottom: 0.5em;
 }
 
-
 h1, h2, h3, h4, h5, h6 {
   font-weight: bold;
   margin: 0px;
@@ -56,22 +54,27 @@ h1 {
   margin-top: 0em;
   font-size: 2em;
 }
+
 h2 {
   margin-top: 1.0em;
   font-size: 1.6em;
 }
+
 h3 {
   margin-top: 1.0em;
   font-size: 1.4em;
 }
+
 h4 {
   margin-top: 1.0em;
   font-size: 1.2em;
 }
+
 h5 {
   margin-top: 0.75em;
   font-size: 1em;
 }
+
 h6 {
   margin-top: 0.75em;
   font-size: 0.8em;
diff --git a/docs/libvirt.css b/docs/libvirt.css
index 17ecbb4..4fffa18 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -2,6 +2,7 @@ h1 {
 font-weight: normal;
 color: #3c857c;
 }
+
 h1 strong {
 font-weight: bold;
 }
@@ -94,6 +95,7 @@ h2, h3, h4, h5, h6 {
 #menu ul.l1 li .inactive {
 border-left: 6px solid #dfebea;
 }
+
 #menu ul.l1 li .active {
 border-left: 6px solid #a5c6c2;
 }
@@ -109,7 +111,6 @@ h2, h3, h4, h5, h6 {
 padding-left: 3em;
 }
 
-
 #headerLogo {
 position: absolute;
 top: 0px;
@@ -118,6 +119,7 @@ h2, h3, h4, h5, h6 {
 width: 400px;
 background: url(libvirt-header-logo.png);
 }
+
 #headerSearch {
 position: absolute;
 top: 0px;
@@ -142,7 +144,6 @@ h2, h3, h4, h5, h6 {
 font-size: 1em;
 }
 
-
 #sitemap ul li {
 list-style: none;
 }
@@ -177,7 +178,6 @@ a {
 color: #566866;
 }
 
-
 div.api {
 border: 1px solid #99;
 background: #ee;
@@ -203,7 +203,6 @@ div.api table td, div.variablelist table td {
 padding-left: 1em;
 }
 
-
 h1 a, h2 a, h3 a, h4 a, h5 a {
 color: inherit;
 text-decoration: inherit;
@@ -252,7 +251,6 @@ p.image {
 width: 100%;
 }
 
-
 #projects dl {
 margin: 0px;
 border: 0px solid white;
@@ -285,9 +283,11 @@ p.image {
 #projects #p1 dt, #projects #p1 dd {
 width: 33%;
 }
+
 #projects #p2 dt, #projects #p2 dd {
 width: 50%;
 }
+
 #projects #p3 dt, #projects #p3 dd {
 width: 99%;
 }
@@ -318,7 +318,6 @@ p.image {
 color: #ccc;
 }
 
-
 #sponsor {
 color: #757575;
 text-decoration: inherit;
@@ -370,6 +369,7 @@ table.data tr.head th {
 table.data tbody td {
 background: rgb(240,240,240);
 }
+
 table.data tbody td.y {
 background: rgb(220,255,220);
 text-align: center;
-- 
2.5.5

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

[libvirt] [PATCH 4/7] docs: Make menu entries smaller

[Andrea Bolognani]

The menu should not take the focus away from the actual contents.
---
 docs/libvirt.css | 5 -
 1 file changed, 5 deletions(-)

diff --git a/docs/libvirt.css b/docs/libvirt.css
index 637ed35..fdbd99e 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -53,11 +53,6 @@ h2, h3, h4, h5, h6 {
 margin: 0px;
 border: 0px;
 font-weight: bold;
-font-size: 1.2em;
-}
-
-#menu li ul li {
-font-size: 1em;
 }
 
 #menu ul li a, #menu ul li span {
-- 
2.5.5

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

Re: [libvirt] [PATCH 01/15] schema: make pci slot and function optional

[Laine Stump]

On 03/30/2016 05:05 AM, Michal Privoznik wrote:

On 24.03.2016 20:25, Laine Stump wrote:

The pcie-switch-downstream-port and pcie-root-port controllers have
only a single slot, numbered 0, and the greate majority of all guest

s/greate/great/


PCI devices are plugged into function 0 of whatever slot they're
using. The parser makes these optional, setting them to 0 when not
specified, and it's logical for the schema to also make them optional.
---
  docs/schemas/basictypes.rng | 16 ++--
  1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index a83063a..084d403 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -75,12 +75,16 @@
  

  
-
-  
-
-
-  
-
+
+  
+
+  
+
+
+  
+
+  
+
  

  


Well, looking into our PCI address parser, all of the attributes are
optional, even domain and bus.


Right. It's only virsh's validation against the schema that causes problems.


  None of my guests have non-zero domain
actually ;-).


Yep. I'm not sure why we waste so much imaginary ink formatting all 
those "domain='0x'" attributes, especially since there isn't even a 
way to specify a non-0 domain for the guest-side PCI address of a 
device. The only thing we (or qemu) support it for is the *host* side 
PCI address of devices that are being assigned with vfio or legacy KVM 
device assignment.


Of course it's always possible that it may be supported in the future, 
but even if that happens, "0" seems like a pretty safe default that 
could be omitted from the stored/displayed config.



  Your call whether to loosen schema even more or stick with
your approach.


I had done it that way initially (made bus optional as well), then 
backed off but I don't remember why now. I guess because I couldn't 
think of a valid reason why someone would need to specify slot or 
function but leave out bus (and slot 0 of bus 0 is reserved, so can't be 
used. And Cole used a different method of allowing plain "type='pci'/>" in his series to force PCI addresses for devices that may 
otherwise use virtio-mmio (although he may want to do it by simply 
making bus optional as you suggest).


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

Re: [libvirt] [PATCH v4 0/2] persistent live migration with specified XML

[Dmitry Andreev]

ping

On 17.03.2016 19:31, Dmitry Andreev wrote:

v4: wrong param name in commit msg

v3:
  - use shorter name for param and rename args
  - move qemuMigrationCookieAddPersistent out from
qemuMigrationBakeCookie
  - rebase to master

v2: reimplemented with new migration param

Libvirt doesn't allow to specify destination persistent domain
configuration. VIR_MIGRATE_PARAM_DEST_XML migration param is used for
active configuration and persistent configuration is taken from source
domain. The problem is mentioned in this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=835300

This patch-set introduces new migration param VIR_MIGRATE_PARAM_PERSIST_XML
and implements its support in qemu driver.

Dmitry Andreev (2):
   qemuMigrationCookieAddPersistent: move it out and change argument type
   qemu: migration: new migration param for persistent destination XML

  include/libvirt/libvirt-domain.h | 15 ++
  src/qemu/qemu_driver.c   | 12 +---
  src/qemu/qemu_migration.c| 64 
  src/qemu/qemu_migration.h|  2 ++
  4 files changed, 63 insertions(+), 30 deletions(-)



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

Re: [libvirt] [PATCH v4 2/2] qemu: Add support to QXL's max_outputs parameter

[Martin Kletzander]

On Wed, Mar 30, 2016 at 01:32:15PM +0200, Martin Kletzander wrote:

Historically, we added heads=1 to videos, but for example for qxl, we
did not reflect that on the command line.  Implementing that now could
mean that if user were to migrate from older to newer libvirt, the
command-line for qemu would differ.  In order for that not to happen a
migration cookie flag is introduced.

Signed-off-by: Martin Kletzander 
---
src/qemu/qemu_command.c|  8 +++
src/qemu/qemu_migration.c  | 64 --
.../qemuhotplug-agent-nopath.xml   |  5 ++
.../qemuhotplug-hotplug-base+agent-nopath.xml  | 58 
.../qemuxml2argv-video-qxl-heads.args  | 28 ++
.../qemuxml2argv-video-qxl-heads.xml   | 47 
.../qemuxml2argv-video-qxl-noheads.args| 24 
.../qemuxml2argv-video-qxl-noheads.xml | 39 +
tests/qemuxml2argvtest.c   | 16 ++
.../qemuxml2xmlout-video-qxl-heads.xml | 47 
.../qemuxml2xmlout-video-qxl-noheads.xml   | 39 +
tests/qemuxml2xmltest.c|  3 +
12 files changed, 373 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-agent-nopath.xml
create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+agent-nopath.xml


Oh, for ... sake, don't look at the hotplug stuff, consider it not being
part of the series, I removed it from the local index and github repo.


create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 45c5398ad8b9..0f1657f09e99 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3985,6 +3985,14 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
/* QEMU accepts mebibytes for vgamem_mb. */
virBufferAsprintf(, ",vgamem_mb=%u", video->vgamem / 1024);
}
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_MAX_OUTPUTS)) {
+if (video->heads)
+virBufferAsprintf(, ",max_outputs=%u", video->heads);
+} else {
+video->heads = 0;
+}
} else if (video->vram &&
((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
  virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8bc76bf1671d..609252ae3ccc 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -87,6 +87,7 @@ enum qemuMigrationCookieFlags {
QEMU_MIGRATION_COOKIE_FLAG_NBD,
QEMU_MIGRATION_COOKIE_FLAG_STATS,
QEMU_MIGRATION_COOKIE_FLAG_MEMORY_HOTPLUG,
+QEMU_MIGRATION_COOKIE_FLAG_VIDEO_HEADS,

QEMU_MIGRATION_COOKIE_FLAG_LAST
};
@@ -100,7 +101,8 @@ VIR_ENUM_IMPL(qemuMigrationCookieFlag,
  "network",
  "nbd",
  "statistics",
-  "memory-hotplug");
+  "memory-hotplug",
+  "video-heads");

enum qemuMigrationCookieFeatures {
QEMU_MIGRATION_COOKIE_GRAPHICS  = (1 << 
QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
@@ -110,6 +112,7 @@ enum qemuMigrationCookieFeatures {
QEMU_MIGRATION_COOKIE_NBD = (1 << QEMU_MIGRATION_COOKIE_FLAG_NBD),
QEMU_MIGRATION_COOKIE_STATS = (1 << QEMU_MIGRATION_COOKIE_FLAG_STATS),
QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG = (1 << 
QEMU_MIGRATION_COOKIE_FLAG_MEMORY_HOTPLUG),
+QEMU_MIGRATION_COOKIE_VIDEO_HEADS = (1 << 
QEMU_MIGRATION_COOKIE_FLAG_VIDEO_HEADS),
};

typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
@@ -1386,6 +1389,9 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
if (flags & QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG)
mig->flagsMandatory |= QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG;

+if (flags & QEMU_MIGRATION_COOKIE_VIDEO_HEADS)
+mig->flagsMandatory |= QEMU_MIGRATION_COOKIE_VIDEO_HEADS;
+
if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
return -1;

@@ -3132,7 +3138,6 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
if (nmigrate_disks) {
if (has_drive_mirror) {
size_t i, j;
-
/* Check user requested only known disk targets. */
for (i = 0; i < nmigrate_disks; i++) {
for (j = 0; j < vm->def->ndisks; j++) {
@@ -3177,6 +3182,29 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
 

[libvirt] [PATCH v4 1/2] qemu: Check for qxl's max_outputs parameter

[Martin Kletzander]

There is max_outputs parameter for both qxl and qxl-vga and there is no
easy way of saying that we want the capability enabled only if both are
supported.  So let's have two of them and only use them together.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_capabilities.c | 4 
 src/qemu/qemu_capabilities.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 28238435268f..aa8444a60538 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -321,6 +321,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "qxl-vga.vram64_size_mb", /* 215 */
   "chardev-logfile",
   "debug-threads",
+  "qxl.max_outputs",
+  "qxl-vga.max_outputs",
 );


@@ -1659,11 +1661,13 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsVmwareSvga[] = {
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxl[] = {
 { "vgamem_mb", QEMU_CAPS_QXL_VGAMEM },
 { "vram64_size_mb", QEMU_CAPS_QXL_VRAM64 },
+{ "max_outputs", QEMU_CAPS_QXL_MAX_OUTPUTS },
 };

 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = {
 { "vgamem_mb", QEMU_CAPS_QXL_VGA_VGAMEM },
 { "vram64_size_mb", QEMU_CAPS_QXL_VGA_VRAM64 },
+{ "max_outputs", QEMU_CAPS_QXL_VGA_MAX_OUTPUTS },
 };

 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioGpu[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index caf3d1be7be3..705a9bfdf67e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -351,6 +351,8 @@ typedef enum {
 QEMU_CAPS_QXL_VGA_VRAM64, /* -device qxl-vga.vram64_size_mb */
 QEMU_CAPS_CHARDEV_LOGFILE, /* -chardev logfile= */
 QEMU_CAPS_NAME_DEBUG_THREADS, /* Is -name debug-threads= available */
+QEMU_CAPS_QXL_MAX_OUTPUTS, /* -device qxl,max-outputs= */
+QEMU_CAPS_QXL_VGA_MAX_OUTPUTS, /* -device qxl-vga,max-outputs= */

 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
-- 
2.8.0

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

[libvirt] [PATCH v4 0/2] qemu: Support for QXL heads

[Martin Kletzander]

v4:
 - Added test case without "heads="
 - Don't consolidate 'size_t i, j' declarations

v3:
 - https://www.redhat.com/archives/libvir-list/2016-March/msg01198.html
 - rebase on top of current master in order to cleanly apply

v2:
 - don't change heads when migrating
 - https://www.redhat.com/archives/libvir-list/2016-March/msg00811.html

v1:
 - https://www.redhat.com/archives/libvir-list/2016-March/msg00410.html

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


Martin Kletzander (2):
  qemu: Check for qxl's max_outputs parameter
  qemu: Add support to QXL's max_outputs parameter

 src/qemu/qemu_capabilities.c   |  4 ++
 src/qemu/qemu_capabilities.h   |  2 +
 src/qemu/qemu_command.c|  8 +++
 src/qemu/qemu_migration.c  | 64 --
 .../qemuhotplug-agent-nopath.xml   |  5 ++
 .../qemuhotplug-hotplug-base+agent-nopath.xml  | 58 
 .../qemuxml2argv-video-qxl-heads.args  | 28 ++
 .../qemuxml2argv-video-qxl-heads.xml   | 47 
 .../qemuxml2argv-video-qxl-noheads.args| 24 
 .../qemuxml2argv-video-qxl-noheads.xml | 39 +
 tests/qemuxml2argvtest.c   | 16 ++
 .../qemuxml2xmlout-video-qxl-heads.xml | 47 
 .../qemuxml2xmlout-video-qxl-noheads.xml   | 39 +
 tests/qemuxml2xmltest.c|  3 +
 14 files changed, 379 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-agent-nopath.xml
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+agent-nopath.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml

--
2.8.0

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

[libvirt] [PATCH v4 2/2] qemu: Add support to QXL's max_outputs parameter

[Martin Kletzander]

Historically, we added heads=1 to videos, but for example for qxl, we
did not reflect that on the command line.  Implementing that now could
mean that if user were to migrate from older to newer libvirt, the
command-line for qemu would differ.  In order for that not to happen a
migration cookie flag is introduced.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_command.c|  8 +++
 src/qemu/qemu_migration.c  | 64 --
 .../qemuhotplug-agent-nopath.xml   |  5 ++
 .../qemuhotplug-hotplug-base+agent-nopath.xml  | 58 
 .../qemuxml2argv-video-qxl-heads.args  | 28 ++
 .../qemuxml2argv-video-qxl-heads.xml   | 47 
 .../qemuxml2argv-video-qxl-noheads.args| 24 
 .../qemuxml2argv-video-qxl-noheads.xml | 39 +
 tests/qemuxml2argvtest.c   | 16 ++
 .../qemuxml2xmlout-video-qxl-heads.xml | 47 
 .../qemuxml2xmlout-video-qxl-noheads.xml   | 39 +
 tests/qemuxml2xmltest.c|  3 +
 12 files changed, 373 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-agent-nopath.xml
 create mode 100644 
tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+agent-nopath.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-heads.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-noheads.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-heads.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-video-qxl-noheads.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 45c5398ad8b9..0f1657f09e99 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3985,6 +3985,14 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
 /* QEMU accepts mebibytes for vgamem_mb. */
 virBufferAsprintf(, ",vgamem_mb=%u", video->vgamem / 1024);
 }
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_MAX_OUTPUTS)) {
+if (video->heads)
+virBufferAsprintf(, ",max_outputs=%u", video->heads);
+} else {
+video->heads = 0;
+}
 } else if (video->vram &&
 ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
   virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8bc76bf1671d..609252ae3ccc 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -87,6 +87,7 @@ enum qemuMigrationCookieFlags {
 QEMU_MIGRATION_COOKIE_FLAG_NBD,
 QEMU_MIGRATION_COOKIE_FLAG_STATS,
 QEMU_MIGRATION_COOKIE_FLAG_MEMORY_HOTPLUG,
+QEMU_MIGRATION_COOKIE_FLAG_VIDEO_HEADS,

 QEMU_MIGRATION_COOKIE_FLAG_LAST
 };
@@ -100,7 +101,8 @@ VIR_ENUM_IMPL(qemuMigrationCookieFlag,
   "network",
   "nbd",
   "statistics",
-  "memory-hotplug");
+  "memory-hotplug",
+  "video-heads");

 enum qemuMigrationCookieFeatures {
 QEMU_MIGRATION_COOKIE_GRAPHICS  = (1 << 
QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
@@ -110,6 +112,7 @@ enum qemuMigrationCookieFeatures {
 QEMU_MIGRATION_COOKIE_NBD = (1 << QEMU_MIGRATION_COOKIE_FLAG_NBD),
 QEMU_MIGRATION_COOKIE_STATS = (1 << QEMU_MIGRATION_COOKIE_FLAG_STATS),
 QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG = (1 << 
QEMU_MIGRATION_COOKIE_FLAG_MEMORY_HOTPLUG),
+QEMU_MIGRATION_COOKIE_VIDEO_HEADS = (1 << 
QEMU_MIGRATION_COOKIE_FLAG_VIDEO_HEADS),
 };

 typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
@@ -1386,6 +1389,9 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
 if (flags & QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG)
 mig->flagsMandatory |= QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG;

+if (flags & QEMU_MIGRATION_COOKIE_VIDEO_HEADS)
+mig->flagsMandatory |= QEMU_MIGRATION_COOKIE_VIDEO_HEADS;
+
 if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
 return -1;

@@ -3132,7 +3138,6 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
 if (nmigrate_disks) {
 if (has_drive_mirror) {
 size_t i, j;
-
 /* Check user requested only known disk targets. */
 for (i = 0; i < nmigrate_disks; i++) {
 for (j = 0; j < vm->def->ndisks; j++) {
@@ -3177,6 +3182,29 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
  vm->newDef && virDomainDefHasMemoryHotplug(vm->newDef)))
 cookieFlags |= QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG;

+/*
+ * If there is at least one QXL video 

Re: [libvirt] [PATCH 0/3] qemu: fix automatic NUMA pinning with memory hotplug

[John Ferlan]

On 03/29/2016 10:52 AM, Peter Krempa wrote:
> We forgot to propagate the automatic nodeset and reported non-ideal errors.
> 
> Peter Krempa (3):
>   qemu: command: Pass numad nodeset when formatting memory devices at
> boot
>   qemu: command: Split up formatting of -numa and memory devices
>   qemu: hotplug: Improve error when hotpluging memory with
> auto-placement
> 
>  src/conf/numa_conf.c | 20 +---
>  src/conf/numa_conf.h |  1 +
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_command.c  | 27 ++-
>  src/qemu/qemu_hotplug.c  |  7 +++
>  5 files changed, 40 insertions(+), 16 deletions(-)
> 

ACK series (seems safe to me),

John

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

Re: [libvirt] [Qemu-devel] [PATCH v7 0/4] ARM: add query-gic-capabilities QMP command

[Andrea Bolognani]

On Tue, 2016-03-29 at 21:10 +0100, Peter Maydell wrote:
> On 24 March 2016 at 02:55, Peter Xu  wrote:
> > 
> > This patch is to add ARM-specific command "query-gic-capability".
> > 
> > The new command can report which kind of GIC device the host/QEMU
> > support. The returned result is in the form of array.
> > 
> > Sample command and output:
> > 
> > {"execute": "query-gic-capability"}
> > {"return": [{"emulated": false, "version": 3, "kernel": false},
> > {"emulated": true, "version": 2, "kernel": true}]}
> 
> I really strongly want this in 2.6 but we are very very nearly
> out of time. Eric or Markus, could I trouble one of you to
> provide an ack for this series for the QMP protocol aspects
> sometime before end of Wednesday, please?
> 
> I also don't think I've seen an indication from the libvirt
> folks that this is definitely the API they want; did I miss it?

Yes, this works nicely for libvirt; in fact, I've already
posted an RFC implementation[1] to the list last week.

Cheers.


[1] https://www.redhat.com/archives/libvir-list/2016-March/msg00956.html
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features

[Pavel Hrdina]

On Wed, Mar 30, 2016 at 12:05:28PM +0300, Maxim Nestratov wrote:
> 30.03.2016 10:50, Pavel Hrdina пишет:
> > On Tue, Mar 29, 2016 at 07:06:14PM +0300, Maxim Nestratov wrote:
> >> 29.03.2016 16:31, Pavel Hrdina пишет:
> >>> Commit 7068b56c introduced several hyperv features.  Not all hyperv
> >>> features are supported by old enough kernels and we shouldn't allow to
> >>> start a guest if kernel doesn't support any of the hyperv feature.
> >>>
> >>> There is one exception, for backward compatibility we cannot error out
> >>> if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
> >>> reason we ignore invtsc, to not break restoring saved domains with older
> >>> libvirt.
> >>>
> > [...]
> >
> >> Hmm, qemu already checks them and simply ignores most of them and
> >> doesn't prevent guest from starting in case they are not supported and
> >> optional. In case they are reqired it fails. Why should we check them
> >> here? At least we should follow the logic qemu has.
> > Yes, that's true that QEMU do some checks and ignores most of the features
> > missing in kernel, but that's no reason why we should do the same.  Libvirt
> > tries to present in domain XML only those features and devices that are 
> > actually
> > present in the guest.  Thus if you tell libvirt that you want some hyperv
> > feature but your host kernel doesn't support it, we should let the user know
> > that this feature isn't supported instead of ignoring that fact and start 
> > the
> > guest anyway.
> >
> > Pavel
> I see your point. And what if a user wants to define some features just 
> to ask libvirt/qemu to do its best if possible and ignore if it isn't 
> without redefining domain xml? Does such case make sense?
> 
> Maxim

Sure, this makes sense and we do this for cpu features [1], for example:



But I'm not that sure whether we want to do this also for guest features like
those hyperv features.  If your guest OS is Windows you probably want to know
what features are supported and to be sure, that all of the specified features
are enabled.

[1] http://libvirt.org/formatdomain.html#elementsCPU

Pavel

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

[libvirt] [PATCH] xen: Also add sub-type for driver=tap2 in xen-xm

[Philipp Hahn]

tap2 only handles 'aio', but not 'raw', which must be explicitly given:

| $ virsh domxml-to-native yyy.xml > yyy.xm
| $ xm new yyy.xm
| Error: tap:/srv/xen/xxx.img not a valid disk type
| $ sed -i -e 's/tap2:/:/' yyy.xm
| $ xm new yyy.xm

Fix reading and writing "xen-xm" format for "tap2" by handling it the
same as "tap".
---
 src/xenconfig/xen_xm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c
index e09d97e..6556886 100644
--- a/src/xenconfig/xen_xm.c
+++ b/src/xenconfig/xen_xm.c
@@ -196,7 +196,8 @@ xenParseXMDisk(virConfPtr conf, virDomainDefPtr def)
 }
 
 /* And the sub-type for tap:XXX: type */
-if (STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap")) {
+if (STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap") ||
+STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap2")) {
 char *driverType;
 
 if (!(tmp = strchr(src, ':')))
@@ -298,7 +299,7 @@ xenFormatXMDisk(virConfValuePtr list,
 else
 type = virStorageFileFormatTypeToString(format);
 virBufferAsprintf(, "%s:", driver);
-if (STREQ(driver, "tap"))
+if (STREQ(driver, "tap") || STREQ(driver, "tap2"))
 virBufferAsprintf(, "%s:", type);
 } else {
 switch (virDomainDiskGetType(disk)) {
-- 
2.1.4

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

Re: [libvirt] [PATCH V4 4/6] libxl: support creating domain with VF assignment from a pool

[Chun Yan Liu]

>>> On 3/29/2016 at 08:05 AM, in message <56f9c6d0.5000...@suse.com>, Jim Fehlig
 wrote: 
> On 03/21/2016 02:11 AM, Chunyan Liu wrote: 
> > Add codes to support creating domain with network defition of assigning 
> > SRIOV VF from a pool. 
> > 
> > Signed-off-by: Chunyan Liu  
> > --- 
> >  src/libxl/libxl_domain.c | 50  
>  
> >  tests/Makefile.am|  3 +++ 
> >  2 files changed, 53 insertions(+) 
> > 
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c 
> > index c8d09b1..d11bf3a 100644 
> > --- a/src/libxl/libxl_domain.c 
> > +++ b/src/libxl/libxl_domain.c 
> > @@ -36,6 +36,7 @@ 
> >  #include "virtime.h" 
> >  #include "locking/domain_lock.h" 
> >  #include "xen_common.h" 
> > +#include "network/bridge_driver.h" 
> >   
> >  #define VIR_FROM_THIS VIR_FROM_LIBXL 
> >   
> > @@ -764,6 +765,10 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, 
> >  if (net->ifname && 
> >  STRPREFIX(net->ifname, LIBXL_GENERATED_PREFIX_XEN)) 
> >  VIR_FREE(net->ifname); 
> > + 
> > +/* cleanup actual device */ 
> > +virDomainNetRemoveHostdev(vm->def, net); 
> > +networkReleaseActualDevice(vm->def, net); 
> >  } 
> >  } 
> >   
> > @@ -960,6 +965,48 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def,  
> libxl_domain_config *d_config) 
> >  } 
> >  } 
> >   
> > +static int 
> > +libxlNetworkPrepareDevices(virDomainDefPtr def) 
> > +{ 
> > +int ret = -1; 
> > +size_t i; 
> > + 
> > +for (i = 0; i < def->nnets; i++) { 
> > +virDomainNetDefPtr net = def->nets[i]; 
> > +int actualType; 
> > + 
> > +/* If appropriate, grab a physical device from the configured 
> > + * network's pool of devices, or resolve bridge device name 
> > + * to the one defined in the network definition. 
> > + */ 
> > +if (networkAllocateActualDevice(def, net) < 0) 
> > +goto cleanup; 
> > + 
> > +actualType = virDomainNetGetActualType(net); 
> > +if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV && 
> > +net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { 
> > +/* Each type='hostdev' network device must also have a 
> > + * corresponding entry in the hostdevs array. For netdevs 
> > + * that are hardcoded as type='hostdev', this is already 
> > + * done by the parser, but for those allocated from a 
> > + * network / determined at runtime, we need to do it 
> > + * separately. 
> > + */ 
> > +virDomainHostdevDefPtr hostdev =  
> virDomainNetGetActualHostdev(net); 
> > +virDomainHostdevSubsysPCIPtr pcisrc =  
> >source.subsys.u.pci; 
> > + 
> > +if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && 
> > +hostdev->source.subsys.type ==  
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) 
> > +pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; 
> > + 
> > +if (virDomainHostdevInsert(def, hostdev) < 0) 
> > +goto cleanup; 
> > +} 
> > +} 
> > +ret = 0; 
> > + cleanup: 
> > +return ret; 
>  
> Nothing to cleanup here. I think we should just return -1 on failure paths  
> and 0 
> on success. 
>  
> > +} 
> >   
> >  /* 
> >   * Start a domain through libxenlight. 
> > @@ -1036,6 +1083,9 @@ libxlDomainStart(libxlDriverPrivatePtr driver,  
> virDomainObjPtr vm, 
> >  vm, true) < 0) 
> >  goto cleanup; 
> >   
> > +if (libxlNetworkPrepareDevices(vm->def) < 0) 
> > +goto cleanup; 
>  
> I accessed a machine to test this series and have found a few problems. 
>  
> 1. When attaching an SR-IOV vf from a pool, the attach appears successful. 
>I see that xen's pciback driver is bound to the device in the host. I 
>didn't see the device in the guest (could be a guest problem), 

For pv guest, from testing, it is shown in guest. For HVM, seems
xl pci-attach has the same result.

> nor in 
>dumpxml or /var/run/libvirt/libxl/.xml.

Ah, there is a mistake during a review change to original patch 1/6.
Didn't notice that earlier.

In libxlDomainAttachNetDevice:
The if (!ret) is needed, maybe cleanup is not proper.
Since for actual type is hostdev case, after a successful
libxlDomainAttachHostDevice, we need to
update vm->def->nets too.

  cleanup:
 libxl_device_nic_dispose();
- out:
-if (!ret)
+if (!ret) {
 vm->def->nets[vm->def->nnets++] = net;
+} else {
+virDomainNetRemoveHostdev(vm->def, net);
+networkReleaseActualDevice(vm->def, net);
+}

Similar for libxDomainDetachNetDevice cleanup:
Original is still needed.

  cleanup:
 libxl_device_nic_dispose();
-if (!ret)
+if (!ret) {
+networkReleaseActualDevice(vm->def, detach);
 

Re: [libvirt] [PATCH 04/15] schema: allow pci address attributes to be in decimal

[Michal Privoznik]

On 24.03.2016 20:25, Laine Stump wrote:
> This is especially useful for "bus", since the bus of a device's pci
> address is matched to the "index" of a controller to determine which
> bus it will be connected to, and "index" is always specified in
> decimal - being able to specify both in decimal at least makes it
> easier to assure a device is being assigned to the correct bus when it
> is added. For the other attributes, it is just a convenience.
> 
> (MB: the parser already allows for any of these attributes to be given
> in decimal, and there are even examples floating around on the
> internet that give them in decimal rather than hex (written in the
> days before virsh did schema validation on all XML). This only updates
> the schema to match the parser.)
> ---
>  docs/schemas/basictypes.rng | 32 
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
> index 64babaf..e2936d8 100644
> --- a/docs/schemas/basictypes.rng
> +++ b/docs/schemas/basictypes.rng
> @@ -282,24 +282,32 @@
>
>  
>
> -
> -  (0x)?[0-9a-fA-F]{1,4}
> -
> +
>
>
> -
> -  (0x)?[0-9a-fA-F]{1,2}
> -
> +
>
>
> -
> -  (0x)?[0-1]?[0-9a-fA-F]
> -
> +
> +  
> +(0x)?[0-1]?[0-9a-fA-F]
> +  
> +  
> +0
> +31
> +  
> +
>
>
> -
> -  (0x)?[0-7]
> -
> +
> +  
> +(0x)?[0-7]
> +  
> +  
> +0
> +7
> +  
> +
>
>  
>
> 

Okay, but we can never format them in decimal as '12' in decimal is
something different than in hexadecimal.

Michal

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

Re: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features

[Maxim Nestratov]

30.03.2016 10:50, Pavel Hrdina пишет:

On Tue, Mar 29, 2016 at 07:06:14PM +0300, Maxim Nestratov wrote:

29.03.2016 16:31, Pavel Hrdina пишет:

Commit 7068b56c introduced several hyperv features.  Not all hyperv
features are supported by old enough kernels and we shouldn't allow to
start a guest if kernel doesn't support any of the hyperv feature.

There is one exception, for backward compatibility we cannot error out
if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
reason we ignore invtsc, to not break restoring saved domains with older
libvirt.


[...]


Hmm, qemu already checks them and simply ignores most of them and
doesn't prevent guest from starting in case they are not supported and
optional. In case they are reqired it fails. Why should we check them
here? At least we should follow the logic qemu has.

Yes, that's true that QEMU do some checks and ignores most of the features
missing in kernel, but that's no reason why we should do the same.  Libvirt
tries to present in domain XML only those features and devices that are actually
present in the guest.  Thus if you tell libvirt that you want some hyperv
feature but your host kernel doesn't support it, we should let the user know
that this feature isn't supported instead of ignoring that fact and start the
guest anyway.

Pavel
I see your point. And what if a user wants to define some features just 
to ask libvirt/qemu to do its best if possible and ignore if it isn't 
without redefining domain xml? Does such case make sense?


Maxim

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

Re: [libvirt] [PATCH 01/15] schema: make pci slot and function optional

[Michal Privoznik]

On 24.03.2016 20:25, Laine Stump wrote:
> The pcie-switch-downstream-port and pcie-root-port controllers have
> only a single slot, numbered 0, and the greate majority of all guest

s/greate/great/

> PCI devices are plugged into function 0 of whatever slot they're
> using. The parser makes these optional, setting them to 0 when not
> specified, and it's logical for the schema to also make them optional.
> ---
>  docs/schemas/basictypes.rng | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
> index a83063a..084d403 100644
> --- a/docs/schemas/basictypes.rng
> +++ b/docs/schemas/basictypes.rng
> @@ -75,12 +75,16 @@
>  
>
>  
> -
> -  
> -
> -
> -  
> -
> +
> +  
> +
> +  
> +
> +
> +  
> +
> +  
> +
>  
>
>  
> 

Well, looking into our PCI address parser, all of the attributes are
optional, even domain and bus. None of my guests have non-zero domain
actually ;-). Your call whether to loosen schema even more or stick with
your approach.

ACK

Michal

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

Re: [libvirt] [PATCH 00/15] support for pxb and pxb-pcie controllers

[Michal Privoznik]

On 24.03.2016 20:25, Laine Stump wrote:
> These two controllers are used to create a new root bus on a 440fx
> (pxb) or q35 (pxb-pie) virtual machine. There may be other use cases,
> but the main reason for me taking the time to support a separate root
> bus is to have assigned devices be visible in the guest on a different
> NUMA node, so that the guest can be aware of the locality of the
> device wrt CPU and memory that are on different NUMA nodes - although
> you aren't required to, you can add a N subelement to the
> bus'  element to indicate which NUMA node it is on (it's up to
> the management application to place devices on that bus that really
> are on the given NUMA node in the host).
> 
> There are several differences between pxb and pxb-pcie, which are
> detailed in the individual commit log messages, but in short:
> 
> 1) pxb is for 440fx, pxb-pcie for q35 (they *might* work on other
> arches/machinetypes that have a PCI or PCIe bus, but I haven't enabled
> that)
> 
> 2) pxb has an integrate d pci-bridge with 32 slots that are (should
> be) hotplug-capable, while pxb-pcie supplies only a single slot, and
> it will only accept a pcie-root-port (which will then accept a single
> device, hotplug-capable) or a pcie-switch-upstream-port.
> 
> Along the way I encountered a few minor problems/ugliness that I took
> care of in patches 01/15 - 09/15. pxb support is in 10-12, and
> pxb-pcie is in 13-15
> 
> There is a bugzilla record associated with this:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1103314
> 
> Laine Stump (15):
>   schema: make pci slot and function optional
>   schema: rename uint8range/uint24range to uint8/uint24
>   schema: new basic type - uint16
>   schema: allow pci address attributes to be in decimal
>   conf: use #define instead of literal for highest slot in upstream port
>   conf: allow use of slot 0 in a dmi-to-pci-bridge
>   conf/qemu: change the way VIR_PCI_CONNECT_TYPE_* flags work
>   conf: utility function to convert PCI controller model into connect
> type
>   qemu: set PCI controller default modelName in a separate function
>   qemu: add capabilities bit for device "pxb"
>   conf: new pci controller model pci-expander-bus
>   qemu: support new pci controller model "pci-expander-bus"
>   qemu: add capabilities bit for device "pxb-pcie"
>   conf: new pci controller model pcie-expander-bus
>   qemu: support new pci controller model "pcie-expander-bus"
> 
>  docs/formatdomain.html.in  |  74 +++-
>  docs/schemas/basictypes.rng|  63 ++--
>  docs/schemas/domaincommon.rng  |  23 +-
>  docs/schemas/networkcommon.rng |  12 +-
>  docs/schemas/nwfilter.rng  |  16 +-
>  src/bhyve/bhyve_device.c   |  10 +-
>  src/conf/domain_addr.c | 119 +--
>  src/conf/domain_addr.h |  68 ++--
>  src/conf/domain_conf.c |  61 +++-
>  src/conf/domain_conf.h |  11 +-
>  src/libvirt_private.syms   |   1 +
>  src/qemu/qemu_capabilities.c   |   4 +
>  src/qemu/qemu_capabilities.h   |   2 +
>  src/qemu/qemu_command.c|  74 
>  src/qemu/qemu_domain.c |  42 +++
>  src/qemu/qemu_domain_address.c | 305 +---
>  tests/qemucapabilitiesdata/caps_2.4.0-1.caps   |   1 +
>  tests/qemucapabilitiesdata/caps_2.5.0-1.caps   |   1 +
>  tests/qemucapabilitiesdata/caps_2.6.0-1.caps   |   2 +
>  tests/qemucapabilitiesdata/caps_2.6.0-1.replies|   3 +
>  .../qemuxml2argv-aarch64-virtio-pci-default.args   |   2 +-
>  ...l2argv-aarch64-virtio-pci-manual-addresses.args |   2 +-
>  .../qemuxml2argv-pci-expander-bus-bad-machine.xml  | 167 +
>  .../qemuxml2argv-pci-expander-bus-bad-node.xml | 160 +
>  .../qemuxml2argv-pci-expander-bus.args |  87 +
>  .../qemuxml2argv-pci-expander-bus.xml  | 167 +
>  .../qemuxml2argv-pcie-expander-bus-bad-machine.xml |  36 ++
>  .../qemuxml2argv-pcie-expander-bus.args| 123 +++
>  .../qemuxml2argv-pcie-expander-bus.xml | 247 +
>  .../qemuxml2argv-pcie-root-port.args   |   2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args |   2 +-
>  .../qemuxml2argv-pcie-switch-downstream-port.args  |   2 +-
>  .../qemuxml2argv-pcie-switch-upstream-port.args|   2 +-
>  .../qemuxml2argv-pcihole64-q35.args|   2 +-
>  .../qemuxml2argv-q35-pm-disable-fallback.args  |   2 +-
>  .../qemuxml2argv-q35-pm-disable.args   |   2 +-
>  .../qemuxml2argv-q35-usb2-multi.args   |   2 +-
>  .../qemuxml2argv-q35-usb2-reorder.args |   2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-q35-usb2.args  |   2 +-
>  

Re: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features

[Pavel Hrdina]

On Tue, Mar 29, 2016 at 07:06:14PM +0300, Maxim Nestratov wrote:
> 29.03.2016 16:31, Pavel Hrdina пишет:
> > Commit 7068b56c introduced several hyperv features.  Not all hyperv
> > features are supported by old enough kernels and we shouldn't allow to
> > start a guest if kernel doesn't support any of the hyperv feature.
> >
> > There is one exception, for backward compatibility we cannot error out
> > if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
> > reason we ignore invtsc, to not break restoring saved domains with older
> > libvirt.
> >

[...]

> 
> Hmm, qemu already checks them and simply ignores most of them and 
> doesn't prevent guest from starting in case they are not supported and 
> optional. In case they are reqired it fails. Why should we check them 
> here? At least we should follow the logic qemu has.

Yes, that's true that QEMU do some checks and ignores most of the features
missing in kernel, but that's no reason why we should do the same.  Libvirt
tries to present in domain XML only those features and devices that are actually
present in the guest.  Thus if you tell libvirt that you want some hyperv
feature but your host kernel doesn't support it, we should let the user know
that this feature isn't supported instead of ignoring that fact and start the
guest anyway.

Pavel

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

Re: [libvirt] [PATCH] host-validate: Fix suggestion for missing cpu cgroup

[Andrea Bolognani]

On Wed, 2016-03-30 at 08:37 +0200, Peter Krempa wrote:
> On Tue, Mar 29, 2016 at 18:45:02 +0200, Andrea Bolognani wrote:
> > 
> > If the cpu cgroup is not found when validating an host for
> > LXC support, virt-host-validate will suggest to enable the
> > CONFIG_CGROUP_SCHED kconfig option.
> > 
> > The appropriate option is really CONFIG_CGROUP_CPU. The
> > QEMU checks already get that right, so no changes needed.
> > ---
> >  tools/virt-host-validate-lxc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> ACK, safe for freeze.

Pushed, thanks :)

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH v2] nss: FreeBSD support

[Roman Bogorodskiy]

  Michal Privoznik wrote:

> On 29.03.2016 12:08, Roman Bogorodskiy wrote:
> >   Michal Privoznik wrote:
> > 
> >> On 27.03.2016 20:07, Roman Bogorodskiy wrote:
> >>>  * tools/nss/libvirt_nss.[ch]: add BSD-comptabile wrappers and
> >>>register via the nss_module_register() interface
> >>>  * m4/virt-nss.m4: add checks if we're building NSS for FreeBSD
> >>>  * tools/Makefile.am: handle target library name differences, as
> >>>Linux needs libnss_libvirt.so.2 and FreeBSD needs
> >>>nss_libvirt.so.1. Also, different syms files have to be used
> >>>as Linux needs to export all the methods while FreeBSD
> >>>only needs to have nss_module_register()
> >>>  * tests/nsstest.c, tests/nssmock.c: s/__linux__/NSS/
> >>>  * libvirt_nss_bsd.syms: FreeBSD syms file
> >>> ---
> >>>  m4/virt-nss.m4 |  18 +-
> >>>  tests/nssmock.c|   6 +-
> >>>  tests/nsstest.c|   2 +-
> >>>  tools/Makefile.am  |  16 -
> >>>  tools/nss/libvirt_nss.c| 139 
> >>> +++--
> >>>  tools/nss/libvirt_nss.h|   9 +++
> >>>  tools/nss/libvirt_nss_bsd.syms |   9 +++
> >>>  7 files changed, 189 insertions(+), 10 deletions(-)
> >>>  create mode 100644 tools/nss/libvirt_nss_bsd.syms
> >>> index b4a4260..31b1177 100644
> >>> --- a/tests/nssmock.c
> >>> +++ b/tests/nssmock.c
> >>> @@ -20,7 +20,7 @@
> >>>  
> >>>  #include 
> >>>  
> >>> -#ifdef __linux__
> >>> +#ifdef NSS
> >>>  # include 
> >>>  # include 
> >>>  # include 
> >>> @@ -107,7 +107,7 @@ open(const char *path, int flags, ...)
> >>>  va_list ap;
> >>>  mode_t mode;
> >>>  va_start(ap, flags);
> >>> -mode = va_arg(ap, mode_t);
> >>> +mode = va_arg(ap, int);
> >>
> >> Why this change? I mean, even in the context it can be seen that @mode
> >> is type of mode_t.
> > 
> > Yeah, I should have mentioned that in the commit log.
> > 
> > I have an error like this:
> > 
> > In file included from ../gnulib/lib/stdio.h:51:0,
> >  from nssmock.c:24:
> > nssmock.c: In function 'open':
> > nssmock.c:110:27: error: 'mode_t' is promoted to 'int' when passed through 
> > '...' [-Werror]
> >  mode = va_arg(ap, mode_t);
> >^
> > nssmock.c:110:27: note: (so you should pass 'int' not 'mode_t' to 'va_arg')
> > nssmock.c:110:27: note: if this code is reached, the program will abort
> > cc1: all warnings being treated as errors
> > Makefile:4854: recipe for target 'nssmock_la-nssmock.lo' failed
> > 
> > This is with:
> > 
> > gcc version 4.8.5 (FreeBSD Ports Collection)

Adjusted the commit message to reflect s/mode_t/int/ change and pushed;
thanks!

Roman Bogorodskiy

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

Re: [libvirt] [PATCH] host-validate: Improve CPU flags processing

[Andrea Bolognani]

On Wed, 2016-03-30 at 08:36 +0200, Peter Krempa wrote:
> Since you are already using libvirt's utils ...
[...]
> Why not virStringSplit ...
[...]
> And STREQ?

No reason at all, really :)

Will respin.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

[libvirt] [libxl] shutdown a domain before it finishes starting

[Zhangbo (Oscar)]

Hi all:
Suppose we have a guest domain which is pvops, for example, rhel6.4. 

Steps to produce the problem:
1 start the guest by virDomainCreate() 
2 the API returns before the guest domain fully available, which means, the 
disks, network interfaces and some import services are not available inside the 
guest.
3 we call virDomainShutdown() to shutdown the guest.

Expected result:
  The guest got shutdown.

The result in fact:
  Because the guest is not available when we call virDomainShutdown(), it 
couldn't  respond to our 'shutdown' xenstore request, the guest turns on later, 
rather than shutting down.

So , the question is:
   In libxl_driver( xen-hypervisor environment), how can we tell that the 
guest is available or not, and is it suitable to shutdown the guest at that 
moment?


Thanks in advance.
Oscar.


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

[libvirt] build: workaround broken SASL header (again)

[Fabiano Fidêncio]

Compilation for xdg-app failed due to a buggy SASL headers present on
the used runtime (org.gnome.Sdk 3.18).

In file included from rpc/virnetsaslcontext.h:24:0,
 from rpc/virnetsaslcontext.c:25:
/usr/include/sasl/sasl.h:230:38: error: unknown type name 'size_t'
 typedef void *sasl_realloc_t(void *, size_t);
  ^
/usr/include/sasl/sasl.h:235:5: error: unknown type name 'sasl_realloc_t'
 sasl_realloc_t *,
---
 src/rpc/virnetsaslcontext.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rpc/virnetsaslcontext.h b/src/rpc/virnetsaslcontext.h
index 54634d5..edc082f 100644
--- a/src/rpc/virnetsaslcontext.h
+++ b/src/rpc/virnetsaslcontext.h
@@ -21,9 +21,9 @@
 #ifndef __VIR_NET_CLIENT_SASL_CONTEXT_H__
 # define __VIR_NET_CLIENT_SASL_CONTEXT_H__
 
+# include "internal.h"
 # include 
 
-# include "internal.h"
 # include "virobject.h"
 
 typedef struct _virNetSASLContext virNetSASLContext;
-- 
2.7.3

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

Re: [libvirt] [PATCH] host-validate: Fix suggestion for missing cpu cgroup

[Peter Krempa]

On Tue, Mar 29, 2016 at 18:45:02 +0200, Andrea Bolognani wrote:
> If the cpu cgroup is not found when validating an host for
> LXC support, virt-host-validate will suggest to enable the
> CONFIG_CGROUP_SCHED kconfig option.
> 
> The appropriate option is really CONFIG_CGROUP_CPU. The
> QEMU checks already get that right, so no changes needed.
> ---
>  tools/virt-host-validate-lxc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

ACK, safe for freeze.

Peter


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

Re: [libvirt] [PATCH] host-validate: Improve CPU flags processing

[Peter Krempa]

On Tue, Mar 29, 2016 at 18:09:44 +0200, Andrea Bolognani wrote:
> Instead of relying on substring search, tokenize the input
> and process each CPU flag separately. This ensures CPU flag
> detection will continue to work correctly even if we start
> looking for CPU flags whose name might appear as part of
> other CPU flags' names.
> 
> The result of processing is stored in a virBitmap, which
> means we don't have to parse /proc/cpuinfo in its entirety
> for each single CPU flag we want to check.
> 
> Moreover, use of the newly-introduced virHostValidateCPUFlag
> enumeration ensures we don't go looking for random CPU flags
> which might actually be simple typos.
> ---
> The concern was raised by John in
> 
>   https://www.redhat.com/archives/libvir-list/2016-March/msg01301.html
> 
> when discussing a new check on the "sie" CPU flag.
> 
>  tools/virt-host-validate-common.c | 50 
> +--
>  tools/virt-host-validate-common.h | 13 +-
>  tools/virt-host-validate-qemu.c   | 12 --
>  3 files changed, 60 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/virt-host-validate-common.c 
> b/tools/virt-host-validate-common.c
> index 8ebf73e..57fa332 100644
> --- a/tools/virt-host-validate-common.c
> +++ b/tools/virt-host-validate-common.c

[...]

> @@ -188,29 +191,47 @@ int virHostValidateNamespace(const char *hvname,
>  }
>  
>  
> -bool virHostValidateHasCPUFlag(const char *name)
> +virBitmapPtr virHostValidateGetCPUFlags(void)
>  {
> -FILE *fp = fopen("/proc/cpuinfo", "r");
> -bool ret = false;
> +FILE *fp;
> +virBitmapPtr flags;
>  
> -if (!fp)
> -return false;
> +if (!(fp = fopen("/proc/cpuinfo", "r")))
> +return NULL;
> +
> +if (!(flags = virBitmapNewQuiet(VIR_HOST_VALIDATE_CPU_FLAG_LAST)))

Since you are already using libvirt's utils ...

> +return NULL;
>  
>  do {
>  char line[1024];
> +char *saveptr;
> +char *token;
>  
>  if (!fgets(line, sizeof(line), fp))
>  break;
>  
> -if (strstr(line, name)) {
> -ret = true;
> -break;
> +if (!(token = strtok_r(line, " \t\n", )))

Why not virStringSplit ...

> +continue;
> +
> +/* The line we're interested in is marked either as "flags" or
> + * as "Features" depending on the architecture, so check both
> + * prefixes. It's very unlikely that there will be no whitespace
> + * between the line name and the colon, but handle that as well */
> +if (strcmp(token, "flags") && strcmp(token, "flags:") &&

And STREQ?

Peter



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