Re: [libvirt] [PATCH] virsh: add --domain option for domain-to-native

2017-05-29 Thread Julio Faracco 
It would be nice to include the changes into documentation, wouldn't it?
tools/virsh.pod has an entry for example.

2017-05-29 16:40 GMT-03:00 Dan :
> On Mon, May 29, 2017 at 08:53:18PM +0200, Martin Kletzander wrote:
>> On Mon, May 29, 2017 at 02:25:03PM -0400, Dan wrote:
>> > On Tue, May 16, 2017 at 11:41:55AM +0200, Martin Kletzander wrote:
>> > > On Mon, May 15, 2017 at 04:29:48AM -0400, Daniel Liu wrote:
>> > > > Fix bug 835476[1].
>> > >
>> > > It's enough to mention it below (as you did).
>> > >
>> > > > virsh: add [--domain DOMAIN] option to  domxml-to-native DOMAIN COMMAND
>> > >
>> > > This first line is already in the subject.
>> > >
>> > > > Add support for the following syntax:
>> > > > domxml-to-native  { [--domain DOMAIN] | [XML] }, i.e., it 
>> > > > supports
>> > > > either designating domain (domain id, uuid, or name), or path to XML 
>> > > > domain
>> > > > configuration file.
>> > > >
>> > >
>> > > I would reword this a little bit.  How would you feel about something
>> > > along the lines of:
>> > >
>> > >  The option allows someone to run domain-to-native on already existing
>> > >  domain without the need of supplying their XML.  It is basically
>> > >  wrapper around `virsh dumpxml $dom | virsh domxml-to-native /dev/stdin`.
>> > >
>> > >  Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
>> > >
>> > I made changes accordingly in the version 2 of this patch.
>> > > > E.g.:
>> > > > virsh domxml-to-native qemu-argv --domain RHEL7.3   # domain name
>> > > > virsh domxml-to-native qemu-argv --domain 10# domain id
>> > > > virsh domxml-to-native qemu-argv dumped_dom.xml # dumped xml
>> > > >
>> > > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=835476
>> > > > ---
>> > > > tools/virsh-domain.c | 54 
>> > > > ++--
>> > > > 1 file changed, 44 insertions(+), 10 deletions(-)
>> > > >
>> > > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>> > > > index 0d19d0e01..a79fd3ab2 100644
>> > > > --- a/tools/virsh-domain.c
>> > > > +++ b/tools/virsh-domain.c
>> > > > @@ -9840,9 +9840,13 @@ static const vshCmdOptDef opts_domxmltonative[] 
>> > > > = {
>> > > >  .flags = VSH_OFLAG_REQ,
>> > > >  .help = N_("target config data type format")
>> > > > },
>> > > > +{.name = "domain",
>> > > > + .type = VSH_OT_DATA,
>> > > > + .flags = VSH_OFLAG_REQ_OPT,
>> > > > + .help = N_("domain name, id or uuid")
>> > > > +},
>> > > > {.name = "xml",
>> > > >  .type = VSH_OT_DATA,
>> > > > - .flags = VSH_OFLAG_REQ,
>> > > >  .help = N_("xml data file to export from")
>> > > > },
>> > > > {.name = NULL}
>> > > > @@ -9851,30 +9855,60 @@ static const vshCmdOptDef 
>> > > > opts_domxmltonative[] = {
>> > > > static bool
>> > > > cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
>> > > > {
>> > > > -bool ret = true;
>> > > > +bool ret = false;
>> > > > const char *format = NULL;
>> > > > -const char *xmlFile = NULL;
>> > > > -char *configData;
>> > > > -char *xmlData;
>> > > > +const char *domain = NULL;
>> > > > +const char *xml = NULL;
>> > > > +char *xmlData = NULL;
>> > > > +char *configData = NULL;
>> > > > unsigned int flags = 0;
>> > > > virshControlPtr priv = ctl->privData;
>> > > > +virDomainPtr dom = NULL;
>> > > >
>> > > > -if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
>> > > > -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
>> > >
>> > > If this was already there, you could keep using it.  But that's not a
>> > > big deal for me, just some others might not like it.
>> > >
>> > I kept the existing code and only added an additional check for "domain"
>> > in v2, i.e.:
>> >
>> >if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
>> >vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
>> >vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
>> >return false;
>> > > > +if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0)
>> > > > +return false;
>> > > > +
>> > > > +if (vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
>> > > > +return false;
>> > > > +
>> > >
>> > > [1] So here you get the domain name/id/uuid ...
>> > >
>> > > > +if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
>> > > > return false;
>> > > >
>> > > > -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
>> > > > +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
>> > > > +
>> > > > +if (domain)
>> > > > +dom = virshCommandOptDomain(ctl, cmd, );
>> > > > +
>> > >
>> > > ... and here you get the object.  What do you supply as the third
>> > > parameter?  Check what the function does.  There's a leak that you will
>> > > fix by getting rid of the lines above [1].  And just supply NULL here.
>> > >
>> > I did not fully get it by "getting rid of the lines above [1]." But I
>>
>> I meant the three lines before the [1] (I used that number in

Re: [libvirt] [PATCH] virsh: add --domain option for domain-to-native

2017-05-29 Thread Dan 
On Mon, May 29, 2017 at 08:53:18PM +0200, Martin Kletzander wrote:
> On Mon, May 29, 2017 at 02:25:03PM -0400, Dan wrote:
> > On Tue, May 16, 2017 at 11:41:55AM +0200, Martin Kletzander wrote:
> > > On Mon, May 15, 2017 at 04:29:48AM -0400, Daniel Liu wrote:
> > > > Fix bug 835476[1].
> > > 
> > > It's enough to mention it below (as you did).
> > > 
> > > > virsh: add [--domain DOMAIN] option to  domxml-to-native DOMAIN COMMAND
> > > 
> > > This first line is already in the subject.
> > > 
> > > > Add support for the following syntax:
> > > > domxml-to-native  { [--domain DOMAIN] | [XML] }, i.e., it 
> > > > supports
> > > > either designating domain (domain id, uuid, or name), or path to XML 
> > > > domain
> > > > configuration file.
> > > >
> > > 
> > > I would reword this a little bit.  How would you feel about something
> > > along the lines of:
> > > 
> > >  The option allows someone to run domain-to-native on already existing
> > >  domain without the need of supplying their XML.  It is basically
> > >  wrapper around `virsh dumpxml $dom | virsh domxml-to-native /dev/stdin`.
> > > 
> > >  Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
> > > 
> > I made changes accordingly in the version 2 of this patch.
> > > > E.g.:
> > > > virsh domxml-to-native qemu-argv --domain RHEL7.3   # domain name
> > > > virsh domxml-to-native qemu-argv --domain 10# domain id
> > > > virsh domxml-to-native qemu-argv dumped_dom.xml # dumped xml
> > > >
> > > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=835476
> > > > ---
> > > > tools/virsh-domain.c | 54 
> > > > ++--
> > > > 1 file changed, 44 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > > > index 0d19d0e01..a79fd3ab2 100644
> > > > --- a/tools/virsh-domain.c
> > > > +++ b/tools/virsh-domain.c
> > > > @@ -9840,9 +9840,13 @@ static const vshCmdOptDef opts_domxmltonative[] 
> > > > = {
> > > >  .flags = VSH_OFLAG_REQ,
> > > >  .help = N_("target config data type format")
> > > > },
> > > > +{.name = "domain",
> > > > + .type = VSH_OT_DATA,
> > > > + .flags = VSH_OFLAG_REQ_OPT,
> > > > + .help = N_("domain name, id or uuid")
> > > > +},
> > > > {.name = "xml",
> > > >  .type = VSH_OT_DATA,
> > > > - .flags = VSH_OFLAG_REQ,
> > > >  .help = N_("xml data file to export from")
> > > > },
> > > > {.name = NULL}
> > > > @@ -9851,30 +9855,60 @@ static const vshCmdOptDef opts_domxmltonative[] 
> > > > = {
> > > > static bool
> > > > cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
> > > > {
> > > > -bool ret = true;
> > > > +bool ret = false;
> > > > const char *format = NULL;
> > > > -const char *xmlFile = NULL;
> > > > -char *configData;
> > > > -char *xmlData;
> > > > +const char *domain = NULL;
> > > > +const char *xml = NULL;
> > > > +char *xmlData = NULL;
> > > > +char *configData = NULL;
> > > > unsigned int flags = 0;
> > > > virshControlPtr priv = ctl->privData;
> > > > +virDomainPtr dom = NULL;
> > > >
> > > > -if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> > > > -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> > > 
> > > If this was already there, you could keep using it.  But that's not a
> > > big deal for me, just some others might not like it.
> > > 
> > I kept the existing code and only added an additional check for "domain"
> > in v2, i.e.:
> > 
> >if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> >vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
> >vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> >return false;
> > > > +if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0)
> > > > +return false;
> > > > +
> > > > +if (vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> > > > +return false;
> > > > +
> > > 
> > > [1] So here you get the domain name/id/uuid ...
> > > 
> > > > +if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> > > > return false;
> > > >
> > > > -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> > > > +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
> > > > +
> > > > +if (domain)
> > > > +dom = virshCommandOptDomain(ctl, cmd, );
> > > > +
> > > 
> > > ... and here you get the object.  What do you supply as the third
> > > parameter?  Check what the function does.  There's a leak that you will
> > > fix by getting rid of the lines above [1].  And just supply NULL here.
> > > 
> > I did not fully get it by "getting rid of the lines above [1]." But I
> 
> I meant the three lines before the [1] (I used that number in square
> brackets as a link.  That's why I said "Check what the function does".

So I deleted the line checking "domain" leaving origin conditional like
as it was:

 if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
vshCommandOptStringReq(ctl, cmd, "xml", ) <

Re: [libvirt] [PATCH] virsh: add --domain option for domain-to-native

2017-05-29 Thread Martin Kletzander 

On Mon, May 29, 2017 at 02:25:03PM -0400, Dan wrote:

On Tue, May 16, 2017 at 11:41:55AM +0200, Martin Kletzander wrote:

On Mon, May 15, 2017 at 04:29:48AM -0400, Daniel Liu wrote:
> Fix bug 835476[1].

It's enough to mention it below (as you did).

> virsh: add [--domain DOMAIN] option to  domxml-to-native DOMAIN COMMAND

This first line is already in the subject.

> Add support for the following syntax:
> domxml-to-native  { [--domain DOMAIN] | [XML] }, i.e., it supports
> either designating domain (domain id, uuid, or name), or path to XML domain
> configuration file.
>

I would reword this a little bit.  How would you feel about something
along the lines of:

 The option allows someone to run domain-to-native on already existing
 domain without the need of supplying their XML.  It is basically
 wrapper around `virsh dumpxml $dom | virsh domxml-to-native /dev/stdin`.

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


I made changes accordingly in the version 2 of this patch.

> E.g.:
> virsh domxml-to-native qemu-argv --domain RHEL7.3   # domain name
> virsh domxml-to-native qemu-argv --domain 10# domain id
> virsh domxml-to-native qemu-argv dumped_dom.xml # dumped xml
>
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=835476
> ---
> tools/virsh-domain.c | 54 ++--
> 1 file changed, 44 insertions(+), 10 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 0d19d0e01..a79fd3ab2 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -9840,9 +9840,13 @@ static const vshCmdOptDef opts_domxmltonative[] = {
>  .flags = VSH_OFLAG_REQ,
>  .help = N_("target config data type format")
> },
> +{.name = "domain",
> + .type = VSH_OT_DATA,
> + .flags = VSH_OFLAG_REQ_OPT,
> + .help = N_("domain name, id or uuid")
> +},
> {.name = "xml",
>  .type = VSH_OT_DATA,
> - .flags = VSH_OFLAG_REQ,
>  .help = N_("xml data file to export from")
> },
> {.name = NULL}
> @@ -9851,30 +9855,60 @@ static const vshCmdOptDef opts_domxmltonative[] = {
> static bool
> cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
> {
> -bool ret = true;
> +bool ret = false;
> const char *format = NULL;
> -const char *xmlFile = NULL;
> -char *configData;
> -char *xmlData;
> +const char *domain = NULL;
> +const char *xml = NULL;
> +char *xmlData = NULL;
> +char *configData = NULL;
> unsigned int flags = 0;
> virshControlPtr priv = ctl->privData;
> +virDomainPtr dom = NULL;
>
> -if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)

If this was already there, you could keep using it.  But that's not a
big deal for me, just some others might not like it.


I kept the existing code and only added an additional check for "domain"
in v2, i.e.:

   if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
   vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
   vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
   return false;

> +if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0)
> +return false;
> +
> +if (vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> +return false;
> +

[1] So here you get the domain name/id/uuid ...

> +if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> return false;
>
> -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
> +
> +if (domain)
> +dom = virshCommandOptDomain(ctl, cmd, );
> +

... and here you get the object.  What do you supply as the third
parameter?  Check what the function does.  There's a leak that you will
fix by getting rid of the lines above [1].  And just supply NULL here.


I did not fully get it by "getting rid of the lines above [1]." But I


I meant the three lines before the [1] (I used that number in square
brackets as a link.  That's why I said "Check what the function does".


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

Re: [libvirt] [PATCH] virsh: add --domain option for domain-to-native

2017-05-29 Thread Dan 
On Tue, May 16, 2017 at 11:41:55AM +0200, Martin Kletzander wrote:
> On Mon, May 15, 2017 at 04:29:48AM -0400, Daniel Liu wrote:
> > Fix bug 835476[1].
> 
> It's enough to mention it below (as you did).
> 
> > virsh: add [--domain DOMAIN] option to  domxml-to-native DOMAIN COMMAND
> 
> This first line is already in the subject.
> 
> > Add support for the following syntax:
> > domxml-to-native  { [--domain DOMAIN] | [XML] }, i.e., it supports
> > either designating domain (domain id, uuid, or name), or path to XML domain
> > configuration file.
> > 
> 
> I would reword this a little bit.  How would you feel about something
> along the lines of:
> 
>  The option allows someone to run domain-to-native on already existing
>  domain without the need of supplying their XML.  It is basically
>  wrapper around `virsh dumpxml $dom | virsh domxml-to-native /dev/stdin`.
> 
>  Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
> 
I made changes accordingly in the version 2 of this patch.
> > E.g.:
> > virsh domxml-to-native qemu-argv --domain RHEL7.3   # domain name
> > virsh domxml-to-native qemu-argv --domain 10# domain id
> > virsh domxml-to-native qemu-argv dumped_dom.xml # dumped xml
> > 
> > [1] https://bugzilla.redhat.com/show_bug.cgi?id=835476
> > ---
> > tools/virsh-domain.c | 54 
> > ++--
> > 1 file changed, 44 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 0d19d0e01..a79fd3ab2 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -9840,9 +9840,13 @@ static const vshCmdOptDef opts_domxmltonative[] = {
> >  .flags = VSH_OFLAG_REQ,
> >  .help = N_("target config data type format")
> > },
> > +{.name = "domain",
> > + .type = VSH_OT_DATA,
> > + .flags = VSH_OFLAG_REQ_OPT,
> > + .help = N_("domain name, id or uuid")
> > +},
> > {.name = "xml",
> >  .type = VSH_OT_DATA,
> > - .flags = VSH_OFLAG_REQ,
> >  .help = N_("xml data file to export from")
> > },
> > {.name = NULL}
> > @@ -9851,30 +9855,60 @@ static const vshCmdOptDef opts_domxmltonative[] = {
> > static bool
> > cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
> > {
> > -bool ret = true;
> > +bool ret = false;
> > const char *format = NULL;
> > -const char *xmlFile = NULL;
> > -char *configData;
> > -char *xmlData;
> > +const char *domain = NULL;
> > +const char *xml = NULL;
> > +char *xmlData = NULL;
> > +char *configData = NULL;
> > unsigned int flags = 0;
> > virshControlPtr priv = ctl->privData;
> > +virDomainPtr dom = NULL;
> > 
> > -if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> > -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> 
> If this was already there, you could keep using it.  But that's not a
> big deal for me, just some others might not like it.
> 
I kept the existing code and only added an additional check for "domain"
in v2, i.e.:

if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
return false;
> > +if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0)
> > +return false;
> > +
> > +if (vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> > +return false;
> > +
> 
> [1] So here you get the domain name/id/uuid ...
> 
> > +if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> > return false;
> > 
> > -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> > +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
> > +
> > +if (domain)
> > +dom = virshCommandOptDomain(ctl, cmd, );
> > +
> 
> ... and here you get the object.  What do you supply as the third
> parameter?  Check what the function does.  There's a leak that you will
> fix by getting rid of the lines above [1].  And just supply NULL here.
> 
I did not fully get it by "getting rid of the lines above [1]." But I
made the change as you suggested to use NULL as the third argument.
> Also make sure  is not NULL here, in case someone supplies
> non-existing domain name.
> 
> > +if (!dom && !xml) {
> > +vshError(ctl, _("need either domain (ID, UUID, or name) or domain 
> > XML configuration file path"));
> > return false;
> > +}
> > +
> > +if (dom) {
> > +xmlData = virDomainGetXMLDesc(dom, flags);
> > +if (xmlData == NULL)
> > +goto cleanup;
> > +}
> > +
> > +if (xml) {
> > +if (virFileReadAll(xml, VSH_MAX_XML_FILE, ) < 0)
> > +goto cleanup;
> > +}
> > 
> 
> This ^^ would be more readable as:
> 
> if (dom) {
>   xmlData = blah;
> } else if (xml) {
>   readFile(asdf, );
> }
> 
> if (!xmlData) {
>  vshError(ctl, "%s",
>   _("Either  or --domain must be suppied"));
> 
> 
> Or something in that regard.
> 
Yep, I changed it accordingly.
> >

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

2017-05-29 Thread Daniel Liu 
The option allows someone to run domain-to-native on already existing
 domain without the need of supplying their XML.  It is basically
 wrapper around 'virsh dumpxml  | virsh domxml-to-native /dev/stdin'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
---
 tools/virsh-domain.c | 52 
 1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ccb514ef9..929f9c896 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9848,9 +9848,13 @@ static const vshCmdOptDef opts_domxmltonative[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_("target config data type format")
 },
+{.name = "domain",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ_OPT,
+ .help = N_("domain name, id or uuid")
+},
 {.name = "xml",
  .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
  .help = N_("xml data file to export from")
 },
 {.name = NULL}
@@ -9859,30 +9863,54 @@ static const vshCmdOptDef opts_domxmltonative[] = {
 static bool
 cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
 {
-bool ret = true;
+bool ret = false;
 const char *format = NULL;
-const char *xmlFile = NULL;
-char *configData;
-char *xmlData;
+const char *domain = NULL;
+const char *xml = NULL;
+char *xmlData = NULL;
+char *configData = NULL;
 unsigned int flags = 0;
 virshControlPtr priv = ctl->privData;
+virDomainPtr dom = NULL;
 
 if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
-vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
+vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
+vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
 return false;
 
-if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
+VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
+
+if (domain) {
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+}
+
+if (dom) {
+xmlData = virDomainGetXMLDesc(dom, flags);
+} else if (xml) {
+if (virFileReadAll(xml, VSH_MAX_XML_FILE, ) < 0)
+goto cleanup;
+}
+
+if (!xmlData) {
+vshError(ctl, "%s",
+ _("need either domain (ID, UUID, or name) or domain XML 
configuration file path"));
 return false;
+}
 
-configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, 
flags);
-if (configData != NULL) {
-vshPrint(ctl, "%s", configData);
-VIR_FREE(configData);
+if (!(configData = virConnectDomainXMLToNative(priv->conn, format, 
xmlData, flags))) {
+vshError(ctl, "%s",
+ _("convert from domain XML to native command failed"));
+goto cleanup;
 } else {
-ret = false;
+vshPrint(ctl, "%s", configData);
+ret = true;
 }
 
+ cleanup:
+virshDomainFree(dom);
 VIR_FREE(xmlData);
+VIR_FREE(configData);
 return ret;
 }
 
-- 
2.13.0

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

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

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

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 67f54282a..f6b352b56 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11541,14 +11541,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
 }

 if (!entry->wr_highest_offset_valid) {
-if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_BLOCK 
&&
-disk->src->format != VIR_STORAGE_FILE_RAW) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("failed to query the maximum written offset of "
- "block device '%s'"), disk->dst);
-goto endjob;
-}
-
 info->allocation = entry->physical;
 } else {
 if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_FILE 
&&
-- 
2.12.2

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

[libvirt] [PATCH v2 4/4] security: don't relabel chardev source if virtlogd is used as stdio handler

2017-05-29 Thread Pavel Hrdina 
In the case that virtlogd is used as stdio handler we pass to QEMU
only FD to a PIPE connected to virtlogd instead of the file itself.

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

Signed-off-by: Pavel Hrdina 
---

Notes:
new in v2

 src/lxc/lxc_process.c|  6 ++---
 src/qemu/qemu_security.c |  9 +--
 src/security/security_apparmor.c |  7 --
 src/security/security_dac.c  | 54 +++-
 src/security/security_driver.h   |  6 +++--
 src/security/security_manager.c  | 12 ++---
 src/security/security_manager.h  |  6 +++--
 src/security/security_nop.c  |  6 +++--
 src/security/security_selinux.c  | 53 ++-
 src/security/security_stack.c| 12 ++---
 tests/securityselinuxlabeltest.c |  2 +-
 11 files changed, 127 insertions(+), 46 deletions(-)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index d8727c3b43..2658ea61f8 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -852,7 +852,7 @@ int virLXCProcessStop(virLXCDriverPtr driver,
 }
 
 virSecurityManagerRestoreAllLabel(driver->securityManager,
-  vm->def, false);
+  vm->def, false, false);
 virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
 /* Clear out dynamically assigned labels */
 if (vm->def->nseclabels &&
@@ -1349,7 +1349,7 @@ int virLXCProcessStart(virConnectPtr conn,
 
 VIR_DEBUG("Setting domain security labels");
 if (virSecurityManagerSetAllLabel(driver->securityManager,
-  vm->def, NULL) < 0)
+  vm->def, NULL, false) < 0)
 goto cleanup;
 
 VIR_DEBUG("Setting up consoles");
@@ -1578,7 +1578,7 @@ int virLXCProcessStart(virConnectPtr conn,
 virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED);
 } else {
 virSecurityManagerRestoreAllLabel(driver->securityManager,
-  vm->def, false);
+  vm->def, false, false);
 virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
 /* Clear out dynamically assigned labels */
 if (vm->def->nseclabels &&
diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 61934f9905..6fc3b0bb6e 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -38,6 +38,7 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
 const char *stdin_path)
 {
 int ret = -1;
+qemuDomainObjPrivatePtr priv = vm->privateData;
 
 if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
 virSecurityManagerTransactionStart(driver->securityManager) < 0)
@@ -45,7 +46,8 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
 
 if (virSecurityManagerSetAllLabel(driver->securityManager,
   vm->def,
-  stdin_path) < 0)
+  stdin_path,
+  priv->chardevStdioLogd) < 0)
 goto cleanup;
 
 if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
@@ -65,6 +67,8 @@ qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
 bool migrated)
 {
+qemuDomainObjPrivatePtr priv = vm->privateData;
+
 /* In contrast to qemuSecuritySetAllLabel, do not use
  * secdriver transactions here. This function is called from
  * qemuProcessStop() which is meant to do cleanup after qemu
@@ -73,7 +77,8 @@ qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver,
  * in entering the namespace then. */
 virSecurityManagerRestoreAllLabel(driver->securityManager,
   vm->def,
-  migrated);
+  migrated,
+  priv->chardevStdioLogd);
 }
 
 
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 62672b0af0..5afe0c5c85 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -489,7 +489,9 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr 
ATTRIBUTE_UNUSED,
 
 static int
 AppArmorSetSecurityAllLabel(virSecurityManagerPtr mgr,
-virDomainDefPtr def, const char *stdin_path)
+virDomainDefPtr def,
+const char *stdin_path,
+bool chardevStdioLogd ATTRIBUTE_UNUSED)
 {
 virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def,
 SECURITY_APPARMOR_NAME);
@@ -567,7 +569,8 @@ AppArmorReleaseSecurityLabel(virSecurityManagerPtr mgr 
ATTRIBUTE_UNUSED,
 static int

[libvirt] [PATCH v2 3/4] qemu: propagate chardevStdioLogd to qemuBuildChrChardevStr

2017-05-29 Thread Pavel Hrdina 
Signed-off-by: Pavel Hrdina 
---

Notes:
new in v2

This is not required to fix the issue by the last patch in the series,
however it improves the code that we decide whether to use virtlogd or
not by checking the same variable that is updated while preparing the
guest start.

 src/qemu/qemu_command.c | 132 +++-
 src/qemu/qemu_command.h |   3 +-
 src/qemu/qemu_process.c |   6 ++-
 3 files changed, 93 insertions(+), 48 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 015af1036c..e6c50d1a64 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5043,7 +5043,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
const virDomainChrSourceDef *dev,
const char *alias,
virQEMUCapsPtr qemuCaps,
-   bool nowait)
+   bool nowait,
+   bool chardevStdioLogd)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 bool telnet;
@@ -5081,8 +5082,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
_("append not supported in this QEMU binary"));
 goto cleanup;
 }
-if (qemuBuildChrChardevFileStr(virQEMUCapsGet(qemuCaps, 
QEMU_CAPS_CHARDEV_FILE_APPEND) ?
-   logManager : NULL, cmd, def, ,
+if (qemuBuildChrChardevFileStr(chardevStdioLogd ? logManager : NULL,
+   cmd, def, ,
"path", dev->data.file.path,
"append", dev->data.file.append) < 0)
 goto cleanup;
@@ -5562,8 +5563,9 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager,
 virQEMUDriverConfigPtr cfg,
 virDomainDefPtr def,
 virQEMUCapsPtr qemuCaps,
-const virDomainChrSourceDef *monitor_chr,
-bool monitor_json)
+virDomainChrSourceDefPtr monitor_chr,
+bool monitor_json,
+bool chardevStdioLogd)
 {
 char *chrdev;
 
@@ -5575,7 +5577,8 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager,
 
 if (!(chrdev = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
   monitor_chr, "monitor",
-  qemuCaps, true)))
+  qemuCaps, true,
+  chardevStdioLogd)))
 return -1;
 virCommandAddArg(cmd, "-chardev");
 virCommandAddArg(cmd, chrdev);
@@ -5720,7 +5723,8 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
  const virDomainDef *def,
  virDomainRNGDefPtr rng,
  virQEMUCapsPtr qemuCaps,
- char **chr)
+ char **chr,
+ bool chardevStdioLogd)
 {
 *chr = NULL;
 
@@ -5733,7 +5737,8 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
 case VIR_DOMAIN_RNG_BACKEND_EGD:
 if (!(*chr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
 rng->source.chardev,
-rng->info.alias, qemuCaps, true)))
+rng->info.alias, qemuCaps, true,
+chardevStdioLogd)))
 return -1;
 }
 
@@ -5881,7 +5886,8 @@ qemuBuildRNGCommandLine(virLogManagerPtr logManager,
 virCommandPtr cmd,
 virQEMUDriverConfigPtr cfg,
 const virDomainDef *def,
-virQEMUCapsPtr qemuCaps)
+virQEMUCapsPtr qemuCaps,
+bool chardevStdioLogd)
 {
 size_t i;
 
@@ -5897,7 +5903,8 @@ qemuBuildRNGCommandLine(virLogManagerPtr logManager,
 
 /* possibly add character device for backend */
 if (qemuBuildRNGBackendChrdevStr(logManager, cmd, cfg, def,
- rng, qemuCaps, ) < 0)
+ rng, qemuCaps, ,
+ chardevStdioLogd) < 0)
 return -1;
 
 if (tmp) {
@@ -8256,7 +8263,8 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
   virDomainDefPtr def,
   virDomainNetDefPtr net,
   virQEMUCapsPtr qemuCaps,
-  unsigned int bootindex)
+  unsigned int bootindex,
+  bool chardevStdioLogd)
 {

[libvirt] [PATCH v2 1/4] conf: move seclabel for chardev source to the correct sturcture

2017-05-29 Thread Pavel Hrdina 
Signed-off-by: Pavel Hrdina 
---

Notes:
new in v2

 src/conf/domain_conf.c  | 46 +++--
 src/conf/domain_conf.h  |  9 
 src/security/security_dac.c | 26 ++-
 src/security/security_manager.c |  4 ++--
 src/security/security_selinux.c | 24 +
 5 files changed, 49 insertions(+), 60 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c7e20b8ba1..68dc2832cb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2076,12 +2076,21 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
 
 void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def)
 {
+size_t i;
+
 if (!def)
 return;
 
 virDomainChrSourceDefClear(def);
 virObjectUnref(def->privateData);
 
+if (def->seclabels) {
+for (i = 0; i < def->nseclabels; i++)
+virSecurityDeviceLabelDefFree(def->seclabels[i]);
+VIR_FREE(def->seclabels);
+}
+
+
 VIR_FREE(def);
 }
 
@@ -2150,8 +2159,6 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef 
*src,
 
 void virDomainChrDefFree(virDomainChrDefPtr def)
 {
-size_t i;
-
 if (!def)
 return;
 
@@ -2176,12 +2183,6 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
 virDomainChrSourceDefFree(def->source);
 virDomainDeviceInfoClear(>info);
 
-if (def->seclabels) {
-for (i = 0; i < def->nseclabels; i++)
-virSecurityDeviceLabelDefFree(def->seclabels[i]);
-VIR_FREE(def->seclabels);
-}
-
 VIR_FREE(def);
 }
 
@@ -10688,8 +10689,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 if (chr_def) {
 xmlNodePtr saved_node = ctxt->node;
 ctxt->node = cur;
-if (virSecurityDeviceLabelDefParseXML(_def->seclabels,
-  _def->nseclabels,
+if (virSecurityDeviceLabelDefParseXML(>seclabels,
+  >nseclabels,
   vmSeclabels,
   nvmSeclabels,
   ctxt,
@@ -22399,19 +22400,11 @@ virDomainNetDefFormat(virBufferPtr buf,
  * output at " type='type'>". */
 static int
 virDomainChrSourceDefFormat(virBufferPtr buf,
-virDomainChrDefPtr chr_def,
 virDomainChrSourceDefPtr def,
 bool tty_compat,
 unsigned int flags)
 {
 const char *type = virDomainChrTypeToString(def->type);
-size_t nseclabels = 0;
-virSecurityDeviceLabelDefPtr *seclabels = NULL;
-
-if (chr_def) {
-nseclabels = chr_def->nseclabels;
-seclabels = chr_def->seclabels;
-}
 
 if (!type) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -22449,7 +22442,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
 def->data.file.append != VIR_TRISTATE_SWITCH_ABSENT)
 virBufferAsprintf(buf, " append='%s'",
 virTristateSwitchTypeToString(def->data.file.append));
-virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, 
flags);
+virDomainSourceDefFormatSeclabel(buf, def->nseclabels,
+ def->seclabels, flags);
 }
 break;
 
@@ -22504,7 +22498,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, "data.nix.listen ? "bind" : "connect");
 virBufferEscapeString(buf, " path='%s'", def->data.nix.path);
-virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, 
flags);
+virDomainSourceDefFormatSeclabel(buf, def->nseclabels,
+ def->seclabels, flags);
 }
 break;
 
@@ -22553,7 +22548,7 @@ virDomainChrDefFormat(virBufferPtr buf,
   def->source->type == VIR_DOMAIN_CHR_TYPE_PTY &&
   !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
   def->source->data.file.path);
-if (virDomainChrSourceDefFormat(buf, def, def->source, tty_compat, flags) 
< 0)
+if (virDomainChrSourceDefFormat(buf, def->source, tty_compat, flags) < 0)
 return -1;
 
 /* Format  block */
@@ -22675,7 +22670,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
 break;
 
 case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-if (virDomainChrSourceDefFormat(buf, NULL, def->data.passthru, false,
+if (virDomainChrSourceDefFormat(buf, def->data.passthru, false,
 flags) < 0)
 return -1;
 break;
@@ -22981,7 +22976,7 @@ virDomainRNGDefFormat(virBufferPtr buf,
 
 case VIR_DOMAIN_RNG_BACKEND_EGD:

[libvirt] [PATCH v2 0/4] fix labeling for chardev source path

2017-05-29 Thread Pavel Hrdina 
Pavel Hrdina (4):
  conf: move seclabel for chardev source to the correct sturcture
  qemu: introduce chardevStdioLogd to qemu private data
  qemu: propagate chardevStdioLogd to qemuBuildChrChardevStr
  security: don't relabel chardev source if virtlogd is used as stdio
handler

 src/conf/domain_conf.c   |  46 +++---
 src/conf/domain_conf.h   |   9 +--
 src/lxc/lxc_process.c|   6 +-
 src/qemu/qemu_command.c  | 132 ++-
 src/qemu/qemu_command.h  |   3 +-
 src/qemu/qemu_domain.c   |   6 ++
 src/qemu/qemu_domain.h   |   3 +
 src/qemu/qemu_process.c  |  15 -
 src/qemu/qemu_security.c |   9 ++-
 src/security/security_apparmor.c |   7 ++-
 src/security/security_dac.c  |  74 +++---
 src/security/security_driver.h   |   6 +-
 src/security/security_manager.c  |  16 +++--
 src/security/security_manager.h  |   6 +-
 src/security/security_nop.c  |   6 +-
 src/security/security_selinux.c  |  71 ++---
 src/security/security_stack.c|  12 ++--
 tests/securityselinuxlabeltest.c |   2 +-
 18 files changed, 281 insertions(+), 148 deletions(-)

-- 
2.13.0

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

[libvirt] [PATCH v2 2/4] qemu: introduce chardevStdioLogd to qemu private data

2017-05-29 Thread Pavel Hrdina 
In QEMU driver we can use virtlogd as stdio handler for source backend
of char devices if current QEMU is new enough and it's enabled in
qemu.conf.  We should store this information while starting a guest
because the config option may change while the guest is running.

Signed-off-by: Pavel Hrdina 
---

Notes:
new in v2

 src/qemu/qemu_domain.c  | 6 ++
 src/qemu/qemu_domain.h  | 3 +++
 src/qemu/qemu_process.c | 9 +
 3 files changed, 18 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0a85ee9d74..b0e3df7009 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1873,6 +1873,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
 virBufferEscapeString(buf, "\n",
   priv->channelTargetDir);
 
+if (priv->chardevStdioLogd)
+virBufferAddLit(buf, "");
+
 return 0;
 }
 
@@ -2141,6 +2144,9 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
 if (qemuDomainSetPrivatePathsOld(driver, vm) < 0)
 goto error;
 
+priv->chardevStdioLogd = virXPathBoolean("boolean(./chardevStdioLogd)",
+ ctxt) == 1;
+
 return 0;
 
  error:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index aebd91ad37..9fb7c339a3 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -293,6 +293,9 @@ struct _qemuDomainObjPrivate {
 /* Used when fetching/storing the current 'tls-creds' migration setting */
 /* (not to be saved in our private XML). */
 char *migTLSAlias;
+
+/* If true virtlogd is used as stdio handler for character devices. */
+bool chardevStdioLogd;
 };
 
 # define QEMU_DOMAIN_PRIVATE(vm)   \
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index be031b56b9..77c2e5f6d3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5367,6 +5367,7 @@ qemuProcessPrepareDomain(virConnectPtr conn,
 size_t i;
 char *nodeset = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 virCapsPtr caps;
 
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
@@ -5403,6 +5404,13 @@ qemuProcessPrepareDomain(virConnectPtr conn,
 }
 }
 
+/* Whether we should use virtlogd as stdio handler for character
+ * devices source backend. */
+if (cfg->stdioLogD &&
+virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) {
+priv->chardevStdioLogd = true;
+}
+
 /*
  * Normally PCI addresses are assigned in the virDomainCreate
  * or virDomainDefine methods. We might still need to assign
@@ -5466,6 +5474,7 @@ qemuProcessPrepareDomain(virConnectPtr conn,
  cleanup:
 VIR_FREE(nodeset);
 virObjectUnref(caps);
+virObjectUnref(cfg);
 return ret;
 }
 
-- 
2.13.0

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

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

2017-05-29 Thread Erik Skultety 
On Mon, May 29, 2017 at 02:16:17PM +0200, Michal Privoznik wrote:
> On 05/19/2017 03:16 PM, Erik Skultety wrote:
> > From: "ning.bo" 
> >
> > When a number of SRIOV VFs (up to 128 on Intel XL710) is created:
> > for i in `seq 0 1`; do
> >   echo 63 > /sys/class/net//device/sriov_numvfs
> > done
> >
> > libvirtd will then report "udev_monitor_receive_device returned NULL"
> > error because the netlink socket buffer is not big enough (using GDB on
> > libudev confirmed this with ENOBUFFS) and thus some udev events were
> > dropped. This results in some devices being missing in the nodedev-list
> > output. This patch overrides the system's rmem_max limit but for that,
> > we need to make sure we've got root privileges.
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1450960
> >
> > Signed-off-by: ning.bo 
> > Signed-off-by: Erik Skultety 
> > ---
> > Additionally, we might want to check for the errno, providing a specific
> > message if such issue occurs again in a further non-specified point in time 
> > and
> > return the generic, yet cryptic one for all other cases.
> >
> >  src/node_device/node_device_udev.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/src/node_device/node_device_udev.c 
> > b/src/node_device/node_device_udev.c
> > index 4ecb0b18f..0b3717397 100644
> > --- a/src/node_device/node_device_udev.c
> > +++ b/src/node_device/node_device_udev.c
> > @@ -1728,6 +1728,13 @@ static int nodeStateInitialize(bool privileged,
> >
> >  udev_monitor_enable_receiving(priv->udev_monitor);
> >
> > +/* mimic udevd's behaviour and override the systems rmem_max limit in 
> > case
> > + * there's a significant number of device 'add' events
> > + */
> > +if (geteuid() == 0)
> > +udev_monitor_set_receive_buffer_size(priv->udev_monitor,
> > + 128 * 1024 * 1024);
> > +
> >  /* We register the monitor with the event callback so we are
> >   * notified by udev of device changes before we enumerate existing
> >   * devices because libvirt will simply recreate the device if we
>
> ACK and safe for freeze. Although on my system it works even without the
> check for euid.

Are you sure? Because the session daemon should not have the CAP_NET_ADMIN
capability. Though it's true that since we don't care about the return value
the check is unnecessary I give you that. Still, the check is harmless and we
know that it will fail for non-privileged user, so I'm going to keep it here.

Thanks for review,
Erik

>
> Michal
>
> --
> 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] [PATCH] qemu: Fix serial stub console allocation

2017-05-29 Thread Peter Krempa 
On Mon, May 29, 2017 at 13:11:09 +0200, Erik Skultety wrote:
> When adding the aliased serial stub console, the structure wasn't
> properly allocated (VIR_ALLOC instead of virDomainChrDefNew) which then
> resulted in SIGSEGV in virDomainChrSourceIsEqual during a serial device
> coldplug.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1434278
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/qemu/qemu_hotplug.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 4a7d99725..34ddb95f8 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1810,7 +1810,7 @@ qemuDomainChrPreInsert(virDomainDefPtr vmdef,
>  if (!vmdef->consoles && VIR_ALLOC(vmdef->consoles) < 0)
>  return -1;
>  
> -if (VIR_ALLOC(vmdef->consoles[0]) < 0) {
> +if (!(vmdef->consoles[0] = virDomainChrDefNew(NULL))) {

With this the code will not call qemuDomainChrSourcePrivateNew in the
qemu driver, which is called everywhere. Are you sure this is okay? If
so please add a comment that xmlopt is not necessary here.

Otherwise it should be simple to pass xmlopt here.


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

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

2017-05-29 Thread Michal Privoznik 
On 05/19/2017 03:16 PM, Erik Skultety wrote:
> From: "ning.bo" 
> 
> When a number of SRIOV VFs (up to 128 on Intel XL710) is created:
> for i in `seq 0 1`; do
>   echo 63 > /sys/class/net//device/sriov_numvfs
> done
> 
> libvirtd will then report "udev_monitor_receive_device returned NULL"
> error because the netlink socket buffer is not big enough (using GDB on
> libudev confirmed this with ENOBUFFS) and thus some udev events were
> dropped. This results in some devices being missing in the nodedev-list
> output. This patch overrides the system's rmem_max limit but for that,
> we need to make sure we've got root privileges.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1450960
> 
> Signed-off-by: ning.bo 
> Signed-off-by: Erik Skultety 
> ---
> Additionally, we might want to check for the errno, providing a specific
> message if such issue occurs again in a further non-specified point in time 
> and
> return the generic, yet cryptic one for all other cases.
> 
>  src/node_device/node_device_udev.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/node_device/node_device_udev.c 
> b/src/node_device/node_device_udev.c
> index 4ecb0b18f..0b3717397 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1728,6 +1728,13 @@ static int nodeStateInitialize(bool privileged,
> 
>  udev_monitor_enable_receiving(priv->udev_monitor);
> 
> +/* mimic udevd's behaviour and override the systems rmem_max limit in 
> case
> + * there's a significant number of device 'add' events
> + */
> +if (geteuid() == 0)
> +udev_monitor_set_receive_buffer_size(priv->udev_monitor,
> + 128 * 1024 * 1024);
> +
>  /* We register the monitor with the event callback so we are
>   * notified by udev of device changes before we enumerate existing
>   * devices because libvirt will simply recreate the device if we

ACK and safe for freeze. Although on my system it works even without the
check for euid.

Michal

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

Re: [libvirt] [PATCH] tools: make setvcpus --maximum imply --config

2017-05-29 Thread Martin Kletzander 

On Mon, May 29, 2017 at 02:23:39PM +0800, Chen Hanxiao wrote:

Hi,
  Should  we remove this restriction(--maximum MUST be used with --config)?

Regards,
-Chen



It doesn't make much sense to remove it now.  There is no hypervisor
that allows that and we can remove it in the future.  I'm not saying we
can't remove it, but there is no point in doing that.  However, if we
take your approach, where --maximum implies --config, then we need to
keep that promise in future versions.  And, if any hypervisor adds
support for changing the maximum later on, there would be no way how to
call it from virsh as --maximum would have to imply --config forever.

I think that's kinda what Peter was trying to say.






在2017年05月29日 13:32，Peter Krempa 写道：
On Sat, May 27, 2017 at 15:14:11 +0800, Chen Hanxiao wrote:

From: Chen Hanxiao 

 Currently --maximum was possible if and only if
 --config was specified.

 This patch makes setvcpus --maximum imply --config.


NACK, some hypervisors may actually allow changing of the maximum in
live config, so this would make it impossible to modify that.



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


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

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

2017-05-29 Thread Martin Kletzander 

On Sun, May 28, 2017 at 11:07:41PM -0300, claudioandre...@gmail.com wrote:

From: Claudio André 

It is possible to test libvirt using other distros in Travis via Docker; 
including (but not limited to) Fedora and Ubuntu.
---
Notes:
* see it working at: https://travis-ci.org/claudioandre/libvirt/builds/237015534
* now, I introduced an error: 
https://travis-ci.org/claudioandre/libvirt/builds/237018298
* I'm using Ubuntu 17.04 because I need to pick something. Could be Xenial 
and/or Fedora and/or ...;

* One test is failing in Ubuntu 17.04 in Travis. The error log says:
 -
   TEST: virkmodtest
 Failed to get config
   !... 4   FAIL
   FAIL virkmodtest (exit status: 1)
 -
* Since it is failing, I used the 'allow_failures'.



'modprobe -c' fails for some reason.  Is the user inside the container
root?  I don't think so.  Some more info output could be nice, like
'uname -a', 'id', and so on, so that we can gather as much info about
the system as possible.  At least for people who don't want to pull the
docker image just to try it out.


.travis.yml| 19 +---
tests/travis-ci.sh | 64 ++


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

The less there needs to be in the script the better as with this you
cannot see the configuration from the Web UI, which is sometimes pretty
convenient.


2 files changed, 75 insertions(+), 8 deletions(-)
create mode 100755 tests/travis-ci.sh

diff --git a/.travis.yml b/.travis.yml
index 5a3e765..7b73761 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -62,11 +62,8 @@ git:
before_install:
  - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew install gnutls 
libgcrypt yajl gettext rpcgen ; fi

-# the custom PATH is just to pick up OS-X homebrew & its harmless on Linux
-before_script:
-  - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
./autogen.sh
script:
-  - VIR_TEST_DEBUG=1 make -j3 && make -j3 syntax-check && make -j3 check
+  - tests/travis-ci.sh



Maybe doing the setup part inside the before_script, so that it is not
taken as a part of the output.  I'm not that familiar with Travis, though.

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


# Environments here are run in addition to the main environment defined above
matrix:
@@ -79,10 +76,16 @@ matrix:
  dist: trusty
- compiler: clang
  os: osx
-  script:
-# many unit tests fail & so does syntax-check, so skip for now
-# one day we must fix it though
-- make -j3
+- services: docker
+  env: IMAGE=ubuntu:17.04 CCO=gcc
+  dist: trusty
+- services: docker
+  env: IMAGE=ubuntu:17.04 CCO=clang
+  dist: trusty
+
+  allow_failures:
+- env: IMAGE=ubuntu:17.04 CCO=gcc
+- env: IMAGE=ubuntu:17.04 CCO=clang

after_failure:
  - echo 
''
diff --git a/tests/travis-ci.sh b/tests/travis-ci.sh
new file mode 100755
index 000..d115564
--- /dev/null
+++ b/tests/travis-ci.sh
@@ -0,0 +1,64 @@
+#!/bin/bash -e
+
+function do_Install_Dependencies(){
+echo
+echo '-- Installing Dependencies --'
+
+apt-get update -qq
+apt-get -y -qq install \


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


+build-essential git clang autoconf libtool libcmpicppimpl0 gettext 
\
+xsltproc autopoint libxml2-dev libncurses5-dev libreadline-dev \
+zlib1g-dev libgnutls28-dev libgcrypt11-dev libavahi-client-dev 
libsasl2-dev \
+libxen-dev lvm2 libgcrypt11-dev libparted0-dev libdevmapper-dev 
uuid-dev \
+libudev-dev libpciaccess-dev libcap-ng-dev libnl-3-dev 
libnl-route-3-dev \
+libyajl-dev libpcap0.8-dev libnuma-dev libnetcf-dev libaudit-dev \
+libxml2-utils libapparmor-dev dnsmasq-base librbd-dev 
w3c-markup-validator


If there is no way how to make output of this command collapsed in the
travis output, I suggest (although others might disagree), to create a
wrapper that does something like the following:

COUNTER=0

silent_if_passes() {
   tmp_name="/tmp/$$_$COUNTER_$RANDOM"
   COUNTER=$((COUNTER+1))
   "$@" >$tmp_name.1.txt 2>$tmp_name.2.txt;
   if [[ "$?" -ne "0" ]]; then
   echo "Command $* failed" >&2
   echo "  stdout:" >&2
   cat $tmp_name.1.txt >&2
   cat $tmp_name.2.txt >&2
   fi
}

And then call not really important setup commands with the
silent_if_passes prefix.

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

2017-05-29 Thread Vladik Romanovsky 
---
 node_device.go  | 277 
 node_device_test.go | 111 +
 2 files changed, 388 insertions(+)
 create mode 100644 node_device.go
 create mode 100644 node_device_test.go

diff --git a/node_device.go b/node_device.go
new file mode 100644
index 000..5375d32
--- /dev/null
+++ b/node_device.go
@@ -0,0 +1,277 @@
+/*
+ * This file is part of the libvirt-go-xml project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ */
+
+package libvirtxml
+
+import (
+   "encoding/xml"
+   "fmt"
+)
+
+type NodeDevice struct {
+   Name   string  `xml:"name"`
+   Path   string  `xml:"path,omitempty"`
+   Parent string  `xml:"parent,omitempty"`
+   Driver string  `xml:"driver>name,omitempty"`
+   Capability *CapabilityType `xml:"capability"`
+}
+
+type CapabilityType struct {
+   Type interface{} `xml:"type,attr"`
+}
+
+type IDName struct {
+   ID   string `xml:"id,attr"`
+   Name string `xml:",chardata"`
+}
+
+type PciExpress struct {
+   Links []PciExpressLink `xml:"link"`
+}
+
+type PciExpressLink struct {
+   Validity string  `xml:"validity,attr,omitempty"`
+   Speedfloat64 `xml:"speed,attr,omitempty"`
+   Port int `xml:"port,attr,omitempty"`
+   Widthint `xml:"width,attr,omitempty"`
+}
+
+type IOMMUGroupType struct {
+   Number int `xml:"number,attr"`
+}
+
+type NUMA struct {
+   Node int `xml:"node,attr"`
+}
+
+type PciCapabilityType struct {
+   Domain   int `xml:"domain,omitempty"`
+   Bus  int `xml:"bus,omitempty"`
+   Slot int `xml:"slot,omitempty"`
+   Function int `xml:"function,omitempty"`
+   Product  IDName  `xml:"product,omitempty"`
+   Vendor   IDName  `xml:"vendor,omitempty"`
+   IommuGroup   IOMMUGroupType  `xml:"iommuGroup,omitempty"`
+   Numa NUMA`xml:"numa,omitempty"`
+   PciExpress   PciExpress  `xml:"pci-express,omitempty"`
+   Capabilities []PciCapability `xml:"capability,omitempty"`
+}
+
+type PCIAddress struct {
+   Domain   string `xml:"domain,attr"`
+   Bus  string `xml:"bus,attr"`
+   Slot string `xml:"slot,attr"`
+   Function string `xml:"function,attr"`
+}
+
+type PciCapability struct {
+   Type string   `xml:"type,attr"`
+   Address  []PCIAddress `xml:"address,omitempty"`
+   MaxCount int  `xml:"maxCount,attr,omitempty"`
+}
+
+type SystemHardware struct {
+   Vendor  string `xml:"vendor"`
+   Version string `xml:"version"`
+   Serial  string `xml:"serial"`
+   UUIDstring `xml:"uuid"`
+}
+
+type SystemFirmware struct {
+   Vendor  string `xml:"vendor"`
+   Version string `xml:"version"`
+   ReleaseData string `xml:"release_date"`
+}
+
+type SystemCapabilityType struct {
+   Product  string `xml:"product"`
+   Hardware SystemHardware `xml:"hardware"`
+   Firmware SystemFirmware `xml:"firmware"`
+}
+
+type USBDeviceCapabilityType struct {
+   Bus int`xml:"bus"`
+   Device  int`xml:"device"`
+   Product IDName `xml:"product,omitempty"`
+   Vendor  IDName `xml:"vendor,omitempty"`
+}
+
+type USBCapabilityType struct {
+   Number  int`xml:"number"`
+   Class   int`xml:"class"`
+   Subclassint`xml:"subclass"`
+   Protocolint`xml:"protocol"`
+   Description string `xml:"description,omitempty"`
+}
+
+type NetOffloadFeatures struct {
+   Name string `xml:"number"`
+}
+
+type NetLink struct {
+   State string `xml:"state,attr"`
+   Speed string `xml:"speed,attr,omitempty"`
+}
+
+type NetCapability struct {
+   Type string `xml:"type,attr"`
+}
+
+type

[libvirt] [PATCH] qemu: Fix serial stub console allocation

2017-05-29 Thread Erik Skultety 
When adding the aliased serial stub console, the structure wasn't
properly allocated (VIR_ALLOC instead of virDomainChrDefNew) which then
resulted in SIGSEGV in virDomainChrSourceIsEqual during a serial device
coldplug.

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

Signed-off-by: Erik Skultety 
---
 src/qemu/qemu_hotplug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4a7d99725..34ddb95f8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1810,7 +1810,7 @@ qemuDomainChrPreInsert(virDomainDefPtr vmdef,
 if (!vmdef->consoles && VIR_ALLOC(vmdef->consoles) < 0)
 return -1;
 
-if (VIR_ALLOC(vmdef->consoles[0]) < 0) {
+if (!(vmdef->consoles[0] = virDomainChrDefNew(NULL))) {
 VIR_FREE(vmdef->consoles);
 return -1;
 }
@@ -1841,7 +1841,7 @@ qemuDomainChrInsertPreAllocCleanup(virDomainDefPtr vmdef,
 /* Remove the stub console added by qemuDomainChrPreInsert */
 if (vmdef->nserials == 0 && vmdef->nconsoles == 1 &&
 chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) {
-VIR_FREE(vmdef->consoles[0]);
+virDomainChrDefFree(vmdef->consoles[0]);
 VIR_FREE(vmdef->consoles);
 vmdef->nconsoles = 0;
 }
-- 
2.13.0

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

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

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

ACK

--
Cedric

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

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

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

ACK

--
Cedric

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

Re: [libvirt] [PATCH] virCapabilitiesInitCaches: Don't leak @cpus

2017-05-29 Thread Peter Krempa 
On Mon, May 29, 2017 at 10:10:26 +0200, Michal Privoznik wrote:
> The @cpus is allocated by virFileReadValueBitmap() but never
> freed:
> 
> ==21274== 40 (32 direct, 8 indirect) bytes in 1 blocks are definitely lost in 
> loss record 808 of 1,004
> ==21274==at 0x4C2E080: calloc (vg_replace_malloc.c:711)
> ==21274==by 0x54BA561: virAlloc (viralloc.c:144)
> ==21274==by 0x54BC604: virBitmapNewEmpty (virbitmap.c:126)
> ==21274==by 0x54BD059: virBitmapParseUnlimited (virbitmap.c:570)
> ==21274==by 0x54EECE9: virFileReadValueBitmap (virfile.c:4113)
> ==21274==by 0x5563132: virCapabilitiesInitCaches (capabilities.c:1548)
> ==21274==by 0x2BB86E59: virQEMUCapsInit (qemu_capabilities.c:1132)
> ==21274==by 0x2BBEC067: virQEMUDriverCreateCapabilities (qemu_conf.c:928)
> ==21274==by 0x2BC3DEAA: qemuStateInitialize (qemu_driver.c:845)
> ==21274==by 0x5625AAC: virStateInitialize (libvirt.c:770)
> ==21274==by 0x124519: daemonRunStateInit (libvirtd.c:881)
> ==21274==by 0x554C927: virThreadHelper (virthread.c:206)
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/capabilities.c | 1 +
>  1 file changed, 1 insertion(+)

ACK, safe for freeze


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

[libvirt] [PATCH] virCapabilitiesInitCaches: Don't leak @cpus

2017-05-29 Thread Michal Privoznik 
The @cpus is allocated by virFileReadValueBitmap() but never
freed:

==21274== 40 (32 direct, 8 indirect) bytes in 1 blocks are definitely lost in 
loss record 808 of 1,004
==21274==at 0x4C2E080: calloc (vg_replace_malloc.c:711)
==21274==by 0x54BA561: virAlloc (viralloc.c:144)
==21274==by 0x54BC604: virBitmapNewEmpty (virbitmap.c:126)
==21274==by 0x54BD059: virBitmapParseUnlimited (virbitmap.c:570)
==21274==by 0x54EECE9: virFileReadValueBitmap (virfile.c:4113)
==21274==by 0x5563132: virCapabilitiesInitCaches (capabilities.c:1548)
==21274==by 0x2BB86E59: virQEMUCapsInit (qemu_capabilities.c:1132)
==21274==by 0x2BBEC067: virQEMUDriverCreateCapabilities (qemu_conf.c:928)
==21274==by 0x2BC3DEAA: qemuStateInitialize (qemu_driver.c:845)
==21274==by 0x5625AAC: virStateInitialize (libvirt.c:770)
==21274==by 0x124519: daemonRunStateInit (libvirtd.c:881)
==21274==by 0x554C927: virThreadHelper (virthread.c:206)

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

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index a91a72a35..43b15761a 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1643,5 +1643,6 @@ virCapabilitiesInitCaches(virCapsPtr caps)
 VIR_FREE(path);
 VIR_DIR_CLOSE(dirp);
 virCapsHostCacheBankFree(bank);
+virBitmapFree(cpus);
 return ret;
 }
-- 
2.13.0

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

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

2017-05-29 Thread Erik Skultety 
On Fri, May 19, 2017 at 03:16:26PM +0200, Erik Skultety wrote:
> From: "ning.bo" 
>

Ping?

> When a number of SRIOV VFs (up to 128 on Intel XL710) is created:
> for i in `seq 0 1`; do
>   echo 63 > /sys/class/net//device/sriov_numvfs
> done
>
> libvirtd will then report "udev_monitor_receive_device returned NULL"
> error because the netlink socket buffer is not big enough (using GDB on
> libudev confirmed this with ENOBUFFS) and thus some udev events were
> dropped. This results in some devices being missing in the nodedev-list
> output. This patch overrides the system's rmem_max limit but for that,
> we need to make sure we've got root privileges.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1450960
>
> Signed-off-by: ning.bo 
> Signed-off-by: Erik Skultety 
> ---
> Additionally, we might want to check for the errno, providing a specific
> message if such issue occurs again in a further non-specified point in time 
> and
> return the generic, yet cryptic one for all other cases.
>
>  src/node_device/node_device_udev.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/node_device/node_device_udev.c 
> b/src/node_device/node_device_udev.c
> index 4ecb0b18f..0b3717397 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1728,6 +1728,13 @@ static int nodeStateInitialize(bool privileged,
>
>  udev_monitor_enable_receiving(priv->udev_monitor);
>
> +/* mimic udevd's behaviour and override the systems rmem_max limit in 
> case
> + * there's a significant number of device 'add' events
> + */
> +if (geteuid() == 0)
> +udev_monitor_set_receive_buffer_size(priv->udev_monitor,
> + 128 * 1024 * 1024);
> +
>  /* We register the monitor with the event callback so we are
>   * notified by udev of device changes before we enumerate existing
>   * devices because libvirt will simply recreate the device if we
> --
> 2.13.0
>
> --
> 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] [PATCH] tools: make setvcpus --maximum imply --config

2017-05-29 Thread Chen Hanxiao 
Hi,
   Should  we remove this restriction(--maximum MUST be used with --config)?

Regards,
-Chen





在2017年05月29日 13:32，Peter Krempa 写道：
On Sat, May 27, 2017 at 15:14:11 +0800, Chen Hanxiao wrote:
> From: Chen Hanxiao 
>
>  Currently --maximum was possible if and only if
>  --config was specified.
>
>  This patch makes setvcpus --maximum imply --config.

NACK, some hypervisors may actually allow changing of the maximum in
live config, so this would make it impossible to modify that.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] tools: make setvcpus --maximum imply --config

2017-05-29 Thread Chen Hanxiao 
Hi,
   Should  we remove this restriction(--maximum MUST be used with --config)?

Regards,
-Chen





在2017年05月29日 13:32，Peter Krempa 写道：
On Sat, May 27, 2017 at 15:14:11 +0800, Chen Hanxiao wrote:
> From: Chen Hanxiao 
>
>  Currently --maximum was possible if and only if
>  --config was specified.
>
>  This patch makes setvcpus --maximum imply --config.

NACK, some hypervisors may actually allow changing of the maximum in
live config, so this would make it impossible to modify that.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list