Re: [libvirt] [RFC]Add new mdev interface for QoS

2017-08-07 Thread Kirti Wankhede


On 8/7/2017 1:11 PM, Gao, Ping A wrote:
> 
> On 2017/8/4 5:11, Alex Williamson wrote:
>> On Thu, 3 Aug 2017 20:26:14 +0800
>> "Gao, Ping A"  wrote:
>>
>>> On 2017/8/3 0:58, Alex Williamson wrote:
 On Wed, 2 Aug 2017 21:16:28 +0530
 Kirti Wankhede  wrote:
  
> On 8/2/2017 6:29 PM, Gao, Ping A wrote:  
>> On 2017/8/2 18:19, Kirti Wankhede wrote:
>>> On 8/2/2017 3:56 AM, Alex Williamson wrote:
 On Tue, 1 Aug 2017 13:54:27 +0800
 "Gao, Ping A"  wrote:

> On 2017/7/28 0:00, Gao, Ping A wrote:
>> On 2017/7/27 0:43, Alex Williamson wrote:  
>>> [cc +libvir-list]
>>>
>>> On Wed, 26 Jul 2017 21:16:59 +0800
>>> "Gao, Ping A"  wrote:
>>>  
 The vfio-mdev provide the capability to let different guest share 
 the
 same physical device through mediate sharing, as result it bring a
 requirement about how to control the device sharing, we need a QoS
 related interface for mdev to management virtual device resource.

 E.g. In practical use, vGPUs assigned to different quests almost 
 has
 different performance requirements, some guests may need higher 
 priority
 for real time usage, some other may need more portion of the GPU
 resource to get higher 3D performance, corresponding we can define 
 some
 interfaces like weight/cap for overall budget control, priority for
 single submission control.

 So I suggest to add some common attributes which are vendor 
 agnostic in
 mdev core sysfs for QoS purpose.  
>>> I think what you're asking for is just some standardization of a QoS
>>> attribute_group which a vendor can optionally include within the
>>> existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
>>> transparently enable this, but it really only provides the standard,
>>> all of the support code is left for the vendor.  I'm fine with that,
>>> but of course the trouble with and sort of standardization is 
>>> arriving
>>> at an agreed upon standard.  Are there QoS knobs that are generic
>>> across any mdev device type?  Are there others that are more 
>>> specific
>>> to vGPU?  Are there existing examples of this that we can steal 
>>> their
>>> specification?  
>> Yes, you are right, standardization QoS knobs are exactly what I 
>> wanted.
>> Only when it become a part of the mdev framework and libvirt, then 
>> QoS
>> such critical feature can be leveraged by cloud usage. HW vendor only
>> need to focus on the implementation of the corresponding QoS 
>> algorithm
>> in their back-end driver.
>>
>> Vfio-mdev framework provide the capability to share the device that 
>> lack
>> of HW virtualization support to guests, no matter the device type,
>> mediated sharing actually is a time sharing multiplex method, from 
>> this
>> point of view, QoS can be take as a generic way about how to control 
>> the
>> time assignment for virtual mdev device that occupy HW. As result we 
>> can
>> define QoS knob generic across any device type by this way. Even if 
>> HW
>> has build in with some kind of QoS support, I think it's not a 
>> problem
>> for back-end driver to convert mdev standard QoS definition to their
>> specification to reach the same performance expectation. Seems there 
>> are
>> no examples for us to follow, we need define it from scratch.
>>
>> I proposal universal QoS control interfaces like below:
>>
>> Cap: The cap limits the maximum percentage of time a mdev device can 
>> own
>> physical device. e.g. cap=60, means mdev device cannot take over 60% 
>> of
>> total physical resource.
>>
>> Weight: The weight define proportional control of the mdev device
>> resource between guests, it’s orthogonal with Cap, to target load
>> balancing. E.g. if guest 1 should take double mdev device resource
>> compare with guest 2, need set weight ratio to 2:1.
>>
>> Priority: The guest who has higher priority will get execution first,
>> target to some real time usage and speeding interactive response.
>>
>> Above QoS interfaces cover both overall budget control and single
>> submission control. I will sent out detail design later once get 
>> aligned.  
> Hi Alex,
> Any comments about the interface mentioned above?
 Not really.

 Kirti, are there a

Re: [libvirt] [PATCH] hostdev: display leading zeros of USB vendor/product id's in error messages

2017-08-07 Thread Chen Hanxiao
At 2017-08-03 08:40:48, "John Ferlan"  wrote:
>
>
>On 07/28/2017 04:33 AM, Chen Hanxiao wrote:
>> From: Chen Hanxiao 
>> 
>> Many vendor id's and product id's have leading zeros.
>> Show them in error messages.
>> 
>> Signed-off-by: Chen Hanxiao 
>> ---
>>  src/util/virhostdev.c | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>
>Looking at some other examples...
>
>if (usbsrc->vendor) {
>virBufferAsprintf(buf, "\n", usbsrc->vendor);
>virBufferAsprintf(buf, "\n", usbsrc->product);
>
>and
>
>if (usbdev->vendor >= 0)
>virBufferAsprintf(buf, " vendor='0x%04X'", usbdev->vendor);
>
>if (usbdev->product >= 0)
>virBufferAsprintf(buf, " product='0x%04X'", usbdev->product);
>
>Perhaps the best thing to do is be consistent with all of them...  Could
>take a bit of searching, but cscope's egrep is pretty good w/
>"vendor.*%.*x" (and X).
>
>There's also a usage in libxl_conf, where "%x:%x" is used. So it may be
>best to find all possible print's of vendor and make them all consistent.
>

The %x:%x should be fixed.

x or X just show a different style.
Others like .4x, 04x have the same effect.
Maybe we should leave them untouched.

Regards,
- Chen

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


Re: [libvirt] [PATCH] hyperv: Reduce usage of libxml API functions

2017-08-07 Thread Matthias Bolte
2017-07-28 18:19 GMT+02:00 Sri Ramanujam :
> Slight refactor of the WMI serialization code to minimize mixing
> openwsman and libxml2 APIs. The only usage of libxml2 APIs now is in
> creating CDATA blocks, because the openwsman API does not provide that
> functionality.
> ---
>  src/hyperv/hyperv_wmi.c | 65 
> -
>  1 file changed, 16 insertions(+), 49 deletions(-)

ACK and pushed, thanks. I adjusted the commit message to emphasis that
this commit is about the clang alignment warnings.


-- 
Matthias Bolte
http://photron.blogspot.com

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


Re: [libvirt] [PATCH] hyperv: Correct number of milliseconds in five minutes

2017-08-07 Thread Matthias Bolte
2017-07-28 18:23 GMT+02:00 Sri Ramanujam :
> ---
>  src/hyperv/hyperv_wmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
> index 57125ae..33ad964 100644
> --- a/src/hyperv/hyperv_wmi.c
> +++ b/src/hyperv/hyperv_wmi.c
> @@ -42,7 +42,7 @@
>
>  #define VIR_FROM_THIS VIR_FROM_HYPERV
>
> -#define HYPERV_JOB_TIMEOUT_MS 5000
> +#define HYPERV_JOB_TIMEOUT_MS 30
>
>  VIR_LOG_INIT("hyperv.hyperv_wmi");

ACK and pushed, thanks.

-- 
Matthias Bolte
http://photron.blogspot.com

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


[libvirt] [RESEND] pci: add support for VMD domains

2017-08-07 Thread Jon Derrick
VMD domains start at 0x1, so expand dev->name to fit at least this
many characters.

Signed-off-by: Jon Derrick 
---
 src/util/virpci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 2c1b758..b3afefe 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -50,7 +50,7 @@ VIR_LOG_INIT("util.pci");
 
 #define PCI_SYSFS "/sys/bus/pci/"
 #define PCI_ID_LEN 10   /* " " */
-#define PCI_ADDR_LEN 13 /* ":XX:XX.X" */
+#define PCI_ADDR_LEN 14 /* "X:XX:XX.X" */
 
 VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST,
   "", "2.5", "5", "8", "16")
-- 
2.9.4

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


Re: [libvirt] [PATCH] introduce virConfReadString

2017-08-07 Thread Michal Privoznik
On 08/07/2017 05:42 PM, Ján Tomko wrote:
> Rewrite virConfReadMem to take a null-terminated string.
> All the callers were calling strlen on it anyway.
> ---
>  daemon/libvirtd-config.c |  2 +-
>  src/libvirt_private.syms |  2 +-
>  src/libxl/libxl_driver.c |  4 ++--
>  src/lxc/lxc_native.c |  2 +-
>  src/util/virconf.c   | 16 
>  src/util/virconf.h   |  4 ++--
>  src/vmx/vmx.c|  4 ++--
>  src/xen/xen_driver.c |  2 +-
>  tests/virconftest.c  |  8 
>  tests/xlconfigtest.c |  2 +-
>  tests/xmconfigtest.c |  2 +-
>  11 files changed, 24 insertions(+), 24 deletions(-)

Yup, ACK.

Michal

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

Re: [libvirt] [PATCH] conf: check rombar against VIR_DOMAIN_TRISTATE_SWITCH_ABSENT

2017-08-07 Thread Michal Privoznik
On 08/07/2017 05:30 PM, Pavel Hrdina wrote:
> On Mon, Aug 07, 2017 at 05:06:49PM +0200, Michal Privoznik wrote:
>> On 08/07/2017 04:56 PM, Ján Tomko wrote:
>>> Make the comparison explicit.
>>> ---
>>>  src/conf/domain_conf.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 3cdb5e348..b5ce2ecd9 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -5359,10 +5359,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
>>>  }
>>>  
>>>  if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) &&
>>> -(info->rombar || info->romfile)) {
>>> +(info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
>>>  
>>>  virBufferAddLit(buf, ">> -if (info->rombar) {
>>> +if (info->rombar != VIR_TRISTATE_SWITCH_ABSENT) {
>>>  const char *rombar = 
>>> virTristateSwitchTypeToString(info->rombar);
>>>  
>>>  if (rombar)
>>>
>>
>> I'm not against this patch, it's just that we set ABSENT explicitly to
>> zero value so that we can do shortcuts like this. If we don't want to
>> have them, we ought to remove the explicit value assignment.
> 
> The shortcut is nice, but I don't like it personally.  If the variable
> can contain more than two states I'd rather check it explicitly. 

So what's the point of assigning _ABSENT zero value then?

> That's
> why I prefer (int == 0) over (!int).

Well, if this is a part of bigger statement then yes, for instance:

if (x == 0) {
} else if (x == 1) {
} else {
}

(although, sometimes we might prefer switch() for that). But if it's
just a simple check whether a value was set or is equal to some default
(= if the check is interested in distinguishing just two states anyway),
!var works for me too:

if (x)
   formatToXML(x)

But sure, var == 0 vs !var is a personal preference. The important part
is my first question. If we dislike these shortcuts (in either of their
form), shouldn't we just drop explicit value assignment in the enum?

Michal

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


[libvirt] [PATCH] introduce virConfReadString

2017-08-07 Thread Ján Tomko
Rewrite virConfReadMem to take a null-terminated string.
All the callers were calling strlen on it anyway.
---
 daemon/libvirtd-config.c |  2 +-
 src/libvirt_private.syms |  2 +-
 src/libxl/libxl_driver.c |  4 ++--
 src/lxc/lxc_native.c |  2 +-
 src/util/virconf.c   | 16 
 src/util/virconf.h   |  4 ++--
 src/vmx/vmx.c|  4 ++--
 src/xen/xen_driver.c |  2 +-
 tests/virconftest.c  |  8 
 tests/xlconfigtest.c |  2 +-
 tests/xmconfigtest.c |  2 +-
 11 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
index 6c0f00ed8..db283a41f 100644
--- a/daemon/libvirtd-config.c
+++ b/daemon/libvirtd-config.c
@@ -433,7 +433,7 @@ int daemonConfigLoadData(struct daemonConfig *data,
 virConfPtr conf;
 int ret;
 
-conf = virConfReadMem(filedata, strlen(filedata), 0);
+conf = virConfReadString(filedata, 0);
 if (!conf)
 return -1;
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 32ac0835e..183a9194d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1520,7 +1520,7 @@ virConfGetValueULLong;
 virConfLoadConfig;
 virConfNew;
 virConfReadFile;
-virConfReadMem;
+virConfReadString;
 virConfSetValue;
 virConfTypeFromString;
 virConfTypeToString;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 907f1776f..c261e2155 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2605,14 +2605,14 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn,
 goto cleanup;
 
 if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) {
-if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0)))
+if (!(conf = virConfReadString(nativeConfig, 0)))
 goto cleanup;
 if (!(def = xenParseXL(conf,
cfg->caps,
driver->xmlopt)))
 goto cleanup;
 } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) {
-if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0)))
+if (!(conf = virConfReadString(nativeConfig, 0)))
 goto cleanup;
 
 if (!(def = xenParseXM(conf,
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index 7bcdf1fb0..5fc6e7cda 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -1000,7 +1000,7 @@ lxcParseConfigString(const char *config,
 virConfPtr properties = NULL;
 virConfValuePtr value;
 
-if (!(properties = virConfReadMem(config, 0, VIR_CONF_FLAG_LXC_FORMAT)))
+if (!(properties = virConfReadString(config, VIR_CONF_FLAG_LXC_FORMAT)))
 return NULL;
 
 if (!(vmdef = virDomainDefNew()))
diff --git a/src/util/virconf.c b/src/util/virconf.c
index c1f41b7e1..e505848f1 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -807,27 +807,27 @@ virConfReadFile(const char *filename, unsigned int flags)
 }
 
 /**
- * virConfReadMem:
+ * virConfReadString:
  * @memory: pointer to the content of the configuration file
- * @len: length in byte
  * @flags: combination of virConfFlag(s)
  *
- * Reads a configuration file loaded in memory. The string can be
- * zero terminated in which case @len can be 0
+ * Reads a configuration file loaded in memory. The string must be
+ * zero terminated.
  *
  * Returns a handle to lookup settings or NULL if it failed to
  * parse the content, use virConfFree() to free the data.
  */
 virConfPtr
-virConfReadMem(const char *memory, int len, unsigned int flags)
+virConfReadString(const char *memory, unsigned int flags)
 {
-if ((memory == NULL) || (len < 0)) {
+size_t len;
+
+if (memory == NULL) {
 virConfError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
 return NULL;
 }
-if (len == 0)
-len = strlen(memory);
 
+len = strlen(memory);
 return virConfParse("memory conf", memory, len, flags);
 }
 
diff --git a/src/util/virconf.h b/src/util/virconf.h
index 23fea4900..f7d9a369c 100644
--- a/src/util/virconf.h
+++ b/src/util/virconf.h
@@ -79,8 +79,8 @@ typedef int (*virConfWalkCallback)(const char* name,
 
 virConfPtr virConfNew(void);
 virConfPtr virConfReadFile(const char *filename, unsigned int flags);
-virConfPtr virConfReadMem(const char *memory,
-  int len, unsigned int flags);
+virConfPtr virConfReadString(const char *memory,
+ unsigned int flags);
 int virConfFree(virConfPtr conf);
 void virConfFreeValue(virConfValuePtr val);
 virConfValuePtr virConfGetValue(virConfPtr conf,
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 96507f10f..3e2f4c3e1 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1312,7 +1312,7 @@ virVMXParseConfig(virVMXContext *ctx,
 return NULL;
 }
 
-conf = virConfReadMem(vmx, strlen(vmx), VIR_CONF_FLAG_VMX_FORMAT);
+conf = virConfReadString(vmx, VIR_CONF_FLAG_VMX_FORMAT);
 
 if (conf == NULL)
 return NULL;
@@ -1332,7 +1332,7 @@ 

Re: [libvirt] [PATCH] conf: check rombar against VIR_DOMAIN_TRISTATE_SWITCH_ABSENT

2017-08-07 Thread Pavel Hrdina
On Mon, Aug 07, 2017 at 05:06:49PM +0200, Michal Privoznik wrote:
> On 08/07/2017 04:56 PM, Ján Tomko wrote:
> > Make the comparison explicit.
> > ---
> >  src/conf/domain_conf.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 3cdb5e348..b5ce2ecd9 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -5359,10 +5359,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
> >  }
> >  
> >  if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) &&
> > -(info->rombar || info->romfile)) {
> > +(info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
> >  
> >  virBufferAddLit(buf, " > -if (info->rombar) {
> > +if (info->rombar != VIR_TRISTATE_SWITCH_ABSENT) {
> >  const char *rombar = 
> > virTristateSwitchTypeToString(info->rombar);
> >  
> >  if (rombar)
> > 
> 
> I'm not against this patch, it's just that we set ABSENT explicitly to
> zero value so that we can do shortcuts like this. If we don't want to
> have them, we ought to remove the explicit value assignment.

The shortcut is nice, but I don't like it personally.  If the variable
can contain more than two states I'd rather check it explicitly.  That's
why I prefer (int == 0) over (!int).

Pavel


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

Re: [libvirt] [PATCH] conf: check rombar against VIR_DOMAIN_TRISTATE_SWITCH_ABSENT

2017-08-07 Thread Michal Privoznik
On 08/07/2017 04:56 PM, Ján Tomko wrote:
> Make the comparison explicit.
> ---
>  src/conf/domain_conf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3cdb5e348..b5ce2ecd9 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5359,10 +5359,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
>  }
>  
>  if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) &&
> -(info->rombar || info->romfile)) {
> +(info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
>  
>  virBufferAddLit(buf, " -if (info->rombar) {
> +if (info->rombar != VIR_TRISTATE_SWITCH_ABSENT) {
>  const char *rombar = virTristateSwitchTypeToString(info->rombar);
>  
>  if (rombar)
> 

I'm not against this patch, it's just that we set ABSENT explicitly to
zero value so that we can do shortcuts like this. If we don't want to
have them, we ought to remove the explicit value assignment.

Michal

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

Re: [libvirt] [PATCH] conf: check rombar against VIR_DOMAIN_TRISTATE_SWITCH_ABSENT

2017-08-07 Thread Pavel Hrdina
On Mon, Aug 07, 2017 at 04:56:07PM +0200, Ján Tomko wrote:
> Make the comparison explicit.
> ---
>  src/conf/domain_conf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Pavel Hrdina 


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

[libvirt] [PATCH] conf: check rombar against VIR_DOMAIN_TRISTATE_SWITCH_ABSENT

2017-08-07 Thread Ján Tomko
Make the comparison explicit.
---
 src/conf/domain_conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3cdb5e348..b5ce2ecd9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5359,10 +5359,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 }
 
 if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) &&
-(info->rombar || info->romfile)) {
+(info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
 
 virBufferAddLit(buf, "rombar) {
+if (info->rombar != VIR_TRISTATE_SWITCH_ABSENT) {
 const char *rombar = virTristateSwitchTypeToString(info->rombar);
 
 if (rombar)
-- 
2.13.0

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


Re: [libvirt] [PATCH 0/3] Properly deal with multiple netdevs per PCI device when converting VF <-> PF

2017-08-07 Thread Michal Privoznik
On 08/04/2017 05:36 AM, Laine Stump wrote:
> The commit log of Patch 1 explains the majority of the reason for
> these patches. In short, they disambiguate the multiple netdevs per
> PCI device on the SRIOV PF and VFs of a Mellanox dual port NIC *when
> converting from a VF netdev to a PF netdev and vice versa*. This
> permits us to set the vlan tag and MAC address for the correct VF
> netdev (and detect the online status of the correct PF netdev for that
> VF) when using a VF in macvtap passthrough mode. (in other words, with
> these patches in place, it is possible to use *all* the VF netdevs on
> both ports of a dual port mellanox NIC for macvtap passthrough.)
> 
> These patches *do not* solve the problem of saving/setting the mac
> address/vlan tag for both ports when using a dual port VF for PCI
> device assignment with VFIO (see my RFC email from yesterday). That is
> a much more complex problem. (These patches are a prerequisite to
> anything that might come out of that RFC though).
> 
> Laine Stump (3):
>   util: new function virNetDevGetPhysPortID()
>   util: support matching a phys_port_id in virPCIGetNetName()
>   util: match phys_port_id when converting PF-netdev to/from VF-netdev
> 
>  src/libvirt_private.syms |  1 +
>  src/util/virhostdev.c|  2 +-
>  src/util/virnetdev.c | 84 
> +---
>  src/util/virnetdev.h |  5 +++
>  src/util/virpci.c| 49 ++--
>  src/util/virpci.h|  4 ++-
>  6 files changed, 129 insertions(+), 16 deletions(-)
> 

ACK

Michal

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


[libvirt] [PATCH 1/2] qemu: fix nwfilter deadlock while reverting to snapshot

2017-08-07 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3f65f440d..0b549f20da 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15325,7 +15325,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 virNWFilterReadLockFilterUpdates();
 
 if (!(vm = qemuDomObjFromSnapshot(snapshot)))
-return -1;
+goto cleanup;
 
 cfg = virQEMUDriverGetConfig(driver);
 
-- 
2.13.4

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


[libvirt] [PATCH 2/2] qemu: fix nwfilter deadlock in qemuProcessReconnect

2017-08-07 Thread Pavel Hrdina
The correct lock order is:

  nwfilter driver lock (not used in this code path)
  nwfilter update lock
  virt driver lock (not used in this code path)
  domain object lock

but the current code have this order:

  domain object lock
  nwfilter update lock

Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_process.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0aecce3b1f..fed2bc5882 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6813,8 +6813,6 @@ qemuProcessReconnect(void *opaque)
 if (qemuDomainMasterKeyReadFile(priv) < 0)
 goto error;
 
-virNWFilterReadLockFilterUpdates();
-
 VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name);
 
 /* XXX check PID liveliness & EXE path */
@@ -7043,6 +7041,8 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
 memcpy(data, src, sizeof(*data));
 data->obj = obj;
 
+virNWFilterReadLockFilterUpdates();
+
 /* this lock and reference will be eventually transferred to the thread
  * that handles the reconnect */
 virObjectLock(obj);
@@ -7068,6 +7068,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
 qemuDomainRemoveInactive(src->driver, obj);
 
 virDomainObjEndAPI(&obj);
+virNWFilterUnlockFilterUpdates();
 virObjectUnref(data->conn);
 VIR_FREE(data);
 return -1;
-- 
2.13.4

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


[libvirt] [PATCH 0/2] fix some nwfilter deadlocks

2017-08-07 Thread Pavel Hrdina
Pavel Hrdina (2):
  qemu: fix nwfilter deadlock while reverting to snapshot
  qemu: fix nwfilter deadlock in qemuProcessReconnect

 src/qemu/qemu_driver.c  | 2 +-
 src/qemu/qemu_process.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.13.4

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


Re: [libvirt] [PATCH] tests: add further XML namespace test

2017-08-07 Thread Michal Privoznik
On 08/04/2017 05:52 PM, Daniel P. Berrange wrote:
> Validate that we can pass QEMU command line options using a default
> namespace, instead of a prefixed namespace
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  .../qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args | 27 +++
>  .../qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml  | 30 
> ++
>  tests/qemuxml2argvtest.c   |  1 +
>  3 files changed, 58 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml

ACK

Michal

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


Re: [libvirt] [PATCH v2] docs: make website responsive for mobile devices

2017-08-07 Thread Michal Privoznik
On 08/04/2017 05:19 PM, Daniel P. Berrange wrote:
> The website does not look good in a mobile device as the text is
> far too small and the layout assumes a wide screen.
> 
> Make the style dynamically adapt based on viewport size, so a
> mobile device gets a layout more suited to its dimensions,
> also changing "Learn" to "Docs"
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  docs/main.css   |  1 +
>  docs/mobile.css | 94 
> +
>  docs/page.xsl   | 36 +-
>  3 files changed, 130 insertions(+), 1 deletion(-)
>  create mode 100644 docs/mobile.css

ACK

Michal

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


[libvirt] [PATCH] Update to latest keycodemapdb content

2017-08-07 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange 
---
 src/Makefile.am   | 2 +-
 src/keycodemapdb  | 2 +-
 src/util/virkeycode.c | 5 ++---
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index b8e875482..45b58c0ad 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -252,7 +252,7 @@ util/virkey%.7: util/virkey%.pod
rm -f $@-t1 && \
mv $@-t2 $@
 
-KEYCODES = linux osx atset1 atset2 atset3 xt xtkbd usb win32 rfb
+KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 rfb
 KEYNAMES = linux osx win32
 
 KEYTABLES = \
diff --git a/src/keycodemapdb b/src/keycodemapdb
index 7bf5710b2..cb0a08d47 16
--- a/src/keycodemapdb
+++ b/src/keycodemapdb
@@ -1 +1 @@
-Subproject commit 7bf5710b22aa8d58b7eeaaf3dc6960c26cade4f0
+Subproject commit cb0a08d4726d93922a25e1285d34179ac278ebf8
diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
index e09aaadaa..eda263218 100644
--- a/src/util/virkeycode.c
+++ b/src/util/virkeycode.c
@@ -30,7 +30,6 @@
 #include "virkeycodetable_rfb.h"
 #include "virkeycodetable_usb.h"
 #include "virkeycodetable_win32.h"
-#include "virkeycodetable_xt.h"
 #include "virkeycodetable_xtkbd.h"
 #include "virkeynametable_linux.h"
 #include "virkeynametable_osx.h"
@@ -44,7 +43,8 @@ static const char **virKeymapNames[VIR_KEYCODE_SET_LAST] = {
 
 static const unsigned short *virKeymapValues[VIR_KEYCODE_SET_LAST] = {
 [VIR_KEYCODE_SET_LINUX] = virKeyCodeTable_linux,
-[VIR_KEYCODE_SET_XT] = virKeyCodeTable_xt,
+/* XT is same as AT Set1 - it was included by mistake */
+[VIR_KEYCODE_SET_XT] = virKeyCodeTable_atset1,
 [VIR_KEYCODE_SET_ATSET1] = virKeyCodeTable_atset1,
 [VIR_KEYCODE_SET_ATSET2] = virKeyCodeTable_atset2,
 [VIR_KEYCODE_SET_ATSET3] = virKeyCodeTable_atset3,
@@ -57,7 +57,6 @@ static const unsigned short 
*virKeymapValues[VIR_KEYCODE_SET_LAST] = {
 
 #define VIR_KEYMAP_ENTRY_MAX ARRAY_CARDINALITY(virKeyCodeTable_linux)
 
-verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_xt));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset1));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset2));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset3));
-- 
2.13.3

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


Re: [libvirt] [PATCH] Remove bogus warning about vir$OBJECTGetConnect functions

2017-08-07 Thread Michal Privoznik
On 08/04/2017 05:30 PM, Daniel P. Berrange wrote:
> The API docs for the various vir$OBJECTGetConnect functions
> contain a warning
> 
>   WARNING: When writing libvirt bindings in other languages, do
>   not use this function.  Instead, store the connection and
>   the domain object together.
> 
> There is no reason why language bindings should not use this
> method, and indeed the Perl, Python, and Go bindings all use
> these methods.
> 
> This warning was originally added back in
> 
>   commit 3edb4bc9fb1b451599df58420d61ffd73633cead
>   Author: Daniel Veillard 
>   Date:   Tue Jul 24 15:32:55 2007 +
> 
> * libvirt.spec.in NEWS docs/* po/*: preparing release 0.3.1
> * src/libvirt.c python/generator.py: some cleanup and warnings
>   from Richard W.M. Jones
> 
> IIUC, the rational was that these APIs do not need to be
> directly exposed to the non-C language, as the language
> can expose the same concept itself by storing the original
> virConnectPtr object alongside the virDomainPtr.  There's
> no reason to mandate such an approach though - it is valid
> for languages to expose this directly if that suits their
> needs better.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  src/libvirt-domain-snapshot.c | 6 --
>  src/libvirt-domain.c  | 4 
>  src/libvirt-interface.c   | 4 
>  src/libvirt-network.c | 4 
>  src/libvirt-secret.c  | 3 ---
>  src/libvirt-storage.c | 8 
>  6 files changed, 29 deletions(-)

ACK

Michal

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


[libvirt] [PATCH 2/2] qemuDomainUndefineFlags: unlink nvram file regardless of domain state

2017-08-07 Thread Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1467245

Currently, there's a bug when undefining a domain with NVRAM
store. Basically, the unlink() of the NVRAM store file happens
during the undefine procedure iff domain is inactive. So, if
domain is running and undefine is called the file is left behind.
It won't be removed in the domain cleanup process either
(qemuProcessStop). One of the solutions is to remove if
regardless of the domain state and rely on qemu having the file
opened. This still has a downside that if the domain is defined
back the NVRAM store file is going to be new, any changes to the
current one are lost (just like with any other file that is
deleted while a process has it opened). But is it really a
downside?

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 574c351ae..992ae2a2e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7367,8 +7367,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 }
 }
 
-if (!virDomainObjIsActive(vm) &&
-vm->def->os.loader && vm->def->os.loader->nvram &&
+if (vm->def->os.loader &&
+vm->def->os.loader->nvram &&
 virFileExists(vm->def->os.loader->nvram)) {
 if ((flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
 if (unlink(vm->def->os.loader->nvram) < 0) {
-- 
2.13.0

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


[libvirt] [PATCH 1/2] qemuDomainUndefineFlags: Grab QEMU_JOB_MODIFY

2017-08-07 Thread Michal Privoznik
This API is definitely modifying state of @vm. Therefore it
should grab a job.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3f65f440..574c351ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7325,10 +7325,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
 if (!vm->persistent) {
 virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cannot undefine transient domain"));
-goto cleanup;
+goto endjob;
 }
 
 if (!virDomainObjIsActive(vm) &&
@@ -7338,15 +7341,15 @@ qemuDomainUndefineFlags(virDomainPtr dom,
_("cannot delete inactive domain with %d "
  "snapshots"),
nsnapshots);
-goto cleanup;
+goto endjob;
 }
 if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0)
-goto cleanup;
+goto endjob;
 }
 
 name = qemuDomainManagedSavePath(driver, vm);
 if (name == NULL)
-goto cleanup;
+goto endjob;
 
 if (virFileExists(name)) {
 if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE) {
@@ -7354,13 +7357,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to remove domain managed "
  "save image"));
-goto cleanup;
+goto endjob;
 }
 } else {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Refusing to undefine while domain managed "
  "save image exists"));
-goto cleanup;
+goto endjob;
 }
 }
 
@@ -7372,17 +7375,17 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 virReportSystemError(errno,
  _("failed to remove nvram: %s"),
  vm->def->os.loader->nvram);
-goto cleanup;
+goto endjob;
 }
 } else if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("cannot delete inactive domain with nvram"));
-goto cleanup;
+goto endjob;
 }
 }
 
 if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0)
-goto cleanup;
+goto endjob;
 
 event = virDomainEventLifecycleNewFromObj(vm,
  VIR_DOMAIN_EVENT_UNDEFINED,
@@ -7399,6 +7402,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 qemuDomainRemoveInactive(driver, vm);
 
 ret = 0;
+ endjob:
+qemuDomainObjEndJob(driver, vm);
 
  cleanup:
 VIR_FREE(name);
-- 
2.13.0

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


[libvirt] [PATCH 0/2] qemuDomainUndefineFlags: Two fixes

2017-08-07 Thread Michal Privoznik
*** BLURB HERE ***

Michal Privoznik (2):
  qemuDomainUndefineFlags: Grab QEMU_JOB_MODIFY
  qemuDomainUndefineFlags: unlink nvram file regardless of domain state

 src/qemu/qemu_driver.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

-- 
2.13.0

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


Re: [libvirt] [PATCH 5/8] virVMXParseConfig: Don't leak def->videos

2017-08-07 Thread Michal Privoznik
On 08/04/2017 04:56 PM, Ján Tomko wrote:
> On Fri, Aug 04, 2017 at 04:22:33PM +0200, Michal Privoznik wrote:
>> This function calls virDomainDefAddImplicitDevices() which adds
>> implicit video device to domain definition. However, later in the
>> process the function just ignores this and overwrites the @videos
>> array without prior free.
>>
> 
> We should not be calling virDomainDefAddImplicitDevices before
> all the explicit devices are added to the domain definition.
> 
> What is the point of adding a device just to free it a few lines later?

Because the parsing code expects some controllers to be in place before
it gets to video devices. Just look around the place where
virDomainDefAddImplicitDevices() is called. If you have a bright idea
how to fix it I'm all ears.

Michal

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


Re: [libvirt] [PATCH] docs: Add "PCI topology and hotplug" guidelines

2017-08-07 Thread Andrea Bolognani
On Tue, 2017-07-25 at 12:42 +0200, Andrea Bolognani wrote:
> For all machine types except i440fx, making a guest hotplug
> capable requires some sort of planning. Add some information
> to help users make educated choices when defining the PCI
> topology of guests.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  docs/formatdomain.html.in |   4 +-
>  docs/pci-hotplug.html.in  | 164 
> ++
>  2 files changed, 167 insertions(+), 1 deletion(-)
>  create mode 100644 docs/pci-hotplug.html.in

Anyone? :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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


Re: [libvirt] [RFC]Add new mdev interface for QoS

2017-08-07 Thread Gao, Ping A

On 2017/8/4 5:11, Alex Williamson wrote:
> On Thu, 3 Aug 2017 20:26:14 +0800
> "Gao, Ping A"  wrote:
>
>> On 2017/8/3 0:58, Alex Williamson wrote:
>>> On Wed, 2 Aug 2017 21:16:28 +0530
>>> Kirti Wankhede  wrote:
>>>  
 On 8/2/2017 6:29 PM, Gao, Ping A wrote:  
> On 2017/8/2 18:19, Kirti Wankhede wrote:
>> On 8/2/2017 3:56 AM, Alex Williamson wrote:
>>> On Tue, 1 Aug 2017 13:54:27 +0800
>>> "Gao, Ping A"  wrote:
>>>
 On 2017/7/28 0:00, Gao, Ping A wrote:
> On 2017/7/27 0:43, Alex Williamson wrote:  
>> [cc +libvir-list]
>>
>> On Wed, 26 Jul 2017 21:16:59 +0800
>> "Gao, Ping A"  wrote:
>>  
>>> The vfio-mdev provide the capability to let different guest share 
>>> the
>>> same physical device through mediate sharing, as result it bring a
>>> requirement about how to control the device sharing, we need a QoS
>>> related interface for mdev to management virtual device resource.
>>>
>>> E.g. In practical use, vGPUs assigned to different quests almost has
>>> different performance requirements, some guests may need higher 
>>> priority
>>> for real time usage, some other may need more portion of the GPU
>>> resource to get higher 3D performance, corresponding we can define 
>>> some
>>> interfaces like weight/cap for overall budget control, priority for
>>> single submission control.
>>>
>>> So I suggest to add some common attributes which are vendor 
>>> agnostic in
>>> mdev core sysfs for QoS purpose.  
>> I think what you're asking for is just some standardization of a QoS
>> attribute_group which a vendor can optionally include within the
>> existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
>> transparently enable this, but it really only provides the standard,
>> all of the support code is left for the vendor.  I'm fine with that,
>> but of course the trouble with and sort of standardization is 
>> arriving
>> at an agreed upon standard.  Are there QoS knobs that are generic
>> across any mdev device type?  Are there others that are more specific
>> to vGPU?  Are there existing examples of this that we can steal their
>> specification?  
> Yes, you are right, standardization QoS knobs are exactly what I 
> wanted.
> Only when it become a part of the mdev framework and libvirt, then QoS
> such critical feature can be leveraged by cloud usage. HW vendor only
> need to focus on the implementation of the corresponding QoS algorithm
> in their back-end driver.
>
> Vfio-mdev framework provide the capability to share the device that 
> lack
> of HW virtualization support to guests, no matter the device type,
> mediated sharing actually is a time sharing multiplex method, from 
> this
> point of view, QoS can be take as a generic way about how to control 
> the
> time assignment for virtual mdev device that occupy HW. As result we 
> can
> define QoS knob generic across any device type by this way. Even if HW
> has build in with some kind of QoS support, I think it's not a problem
> for back-end driver to convert mdev standard QoS definition to their
> specification to reach the same performance expectation. Seems there 
> are
> no examples for us to follow, we need define it from scratch.
>
> I proposal universal QoS control interfaces like below:
>
> Cap: The cap limits the maximum percentage of time a mdev device can 
> own
> physical device. e.g. cap=60, means mdev device cannot take over 60% 
> of
> total physical resource.
>
> Weight: The weight define proportional control of the mdev device
> resource between guests, it’s orthogonal with Cap, to target load
> balancing. E.g. if guest 1 should take double mdev device resource
> compare with guest 2, need set weight ratio to 2:1.
>
> Priority: The guest who has higher priority will get execution first,
> target to some real time usage and speeding interactive response.
>
> Above QoS interfaces cover both overall budget control and single
> submission control. I will sent out detail design later once get 
> aligned.  
 Hi Alex,
 Any comments about the interface mentioned above?
>>> Not really.
>>>
>>> Kirti, are there any QoS knobs that would be interesting
>>> for NVIDIA devices?
>>>
>> We have different types of vGPU for different QoS factors.
>>
>> When mdev devices are created, its resources are allocated irrespective