[libvirt] [PATCH] virsh-domain: Free dom before return false in cmdDump
---
tools/virsh-domain.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 856e888..f6d4edd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4487,8 +4487,10 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, name)))
return false;
-if (vshCommandOptStringReq(ctl, cmd, file, to) 0)
+if (vshCommandOptStringReq(ctl, cmd, file, to) 0) {
+virDomainFree(dom);
return false;
+}
if (vshCommandOptBool(cmd, verbose))
verbose = true;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] docs: define style of code blocks inside descriptions
Signed-off-by: Claudio Bley cb...@av-test.de
---
docs/libvirt.css |7 +++
1 file changed, 7 insertions(+)
diff --git a/docs/libvirt.css b/docs/libvirt.css
index ed67b2f..b324ac8 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -491,3 +491,10 @@ table.acl tr, table.acl td {
table.acl thead {
background: #ddd;
}
+
+div.description pre.code {
+border: 1px dashed grey;
+background-color: inherit;
+padding: 5px 10px 5px 10px;
+margin-left: 2.5em;
+}
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] docs: add class description to div's containing descriptions
Signed-off-by: Claudio Bley cb...@av-test.de
---
docs/newapi.xsl |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index d62839a..a08b304 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -416,7 +416,7 @@
xsl:variable name=name select=string(@name)/
h3a name={$name}codexsl:value-of select=$name//code/a/h3
pre class=apispan class=directive#define/spanxsl:text
/xsl:textxsl:value-of select=$name//pre
-div
+div class=description
xsl:call-template name=formattext
xsl:with-param name=text select=info/
/xsl:call-template
@@ -568,7 +568,7 @@
xsl:text)
/xsl:text
/pre
-div
+div class=description
xsl:call-template name=formattext
xsl:with-param name=text select=info/
/xsl:call-template
@@ -644,7 +644,7 @@
/xsl:for-each
xsl:text)/xsl:text
/pre
-div
+div class=description
xsl:call-template name=formattext
xsl:with-param name=text select=info/
/xsl:call-template
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] API documentation improvements
Hi. It's been a while since I tackled this, but here it goes... This is version 4 of https://www.redhat.com/archives/libvir-list/2013-January/msg02094.html exclusive of the already applied patches, of course. Changes since v3: * skipped the ECMAScript code highlighting patch[1] in this series (postponed for now) * added link generation patch (#6) which I had proposed seperately back in Jan 2013, too. * added a reference to an affected API in patch #1 and #4 as per Eric's comments * changed the code block XSL processing to avoid cutting off characters at the beginning of a line [1] https://www.redhat.com/archives/libvir-list/2013-January/msg02104.html [2] https://www.redhat.com/archives/libvir-list/2013-January/msg00884.html Claudio Bley (6): docs: process code blocks similar to Markdown docs: add class description to div's containing descriptions docs: define style of code blocks inside descriptions libvirt.c: add 2 spaces of indentation to example code of virStreamSend libvirt.c: indent code of virDomainGetMemoryParameters's documentation docs: generate links from plain text documentation docs/libvirt.css |7 +++ docs/newapi.xsl | 121 +++--- src/libvirt.c| 130 +++--- 3 files changed, 167 insertions(+), 91 deletions(-) -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] docs: generate links from plain text documentation
There are two forms used throughout libvirt code comments, which
are both supported by this patch.
* plain links like e.g. http://www.libvirt.org/
* links enclosed in characters, e.g. http://www.libvirt.org/
Signed-off-by: Claudio Bley cb...@av-test.de
---
docs/newapi.xsl | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index a08b304..815c1b9 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -129,6 +129,19 @@
a href=libvirt-{$ref/@file}.html#{$ref/@name}xsl:value-of
select=$stem//a
xsl:value-of select=substring-after($token, $stem)/
/xsl:when
+ xsl:when test=starts-with($token, 'http://')
+a href={$token}
+ xsl:value-of select=$token/
+/a
+ /xsl:when
+ xsl:when test=starts-with($token, 'lt;http://') and contains($token,
'gt;')
+xsl:variable name=link
+ select=substring(substring-before($token, 'gt;'), 2)/
+a href={$link}
+ xsl:value-of select=$link/
+/a
+xsl:value-of select=substring-after($token, 'gt;')/
+ /xsl:when
xsl:otherwise
xsl:value-of select=$token/
/xsl:otherwise
@@ -690,7 +703,11 @@
h2 style=font-weight:bold;color:red;text-align:centerThis module is
deprecated/h2
/xsl:if
xsl:if test=description
- pxsl:value-of select=description//p
+ p
+xsl:call-template name=dumptext
+ xsl:with-param name=text select=description/
+/xsl:call-template
+ /p
/xsl:if
/xsl:template
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] docs: process code blocks similar to Markdown
Wrap pre-formatted example code in code elements. This works similar to Markdown[1] code blocks[2]: Every line indented with at least 2 spaces is considered a code block and gets wrapped in pre and code tags. Look at the documentation for e.g. virStreamSend for before-and-after effects. [1] http://daringfireball.net/projects/markdown/ [2] http://daringfireball.net/projects/markdown/syntax#precode Signed-off-by: Claudio Bley cb...@av-test.de --- docs/newapi.xsl | 96 ++- 1 file changed, 74 insertions(+), 22 deletions(-) diff --git a/docs/newapi.xsl b/docs/newapi.xsl index 7fa0f26..d62839a 100644 --- a/docs/newapi.xsl +++ b/docs/newapi.xsl @@ -150,6 +150,67 @@ /xsl:for-each /xsl:template + + !-- process blocks of text. blocks are separated by two consecutive line -- + !-- breaks. -- + !-- -- + !-- blocks indented with at least 2 spaces are considered code blocks. -- + !-- -- + !-- consecutive code blocks are collapsed into a single code block. -- + xsl:template name=formatblock +xsl:param name=block/ +xsl:param name=rest/ + +xsl:variable name=multipleCodeBlocks + select=starts-with($block, ' ') and starts-with($rest, ' ')/ + +xsl:choose + xsl:when test=$multipleCodeBlocks +xsl:call-template name=formatblock + xsl:with-param name=block +xsl:choose + xsl:when test=contains($rest, '#xA;#xA;') +xsl:value-of select=concat($block, '#xA; #xA;', +substring-before($rest, '#xA;#xA;')) / + /xsl:when + xsl:otherwise +xsl:value-of select=concat($block, '#xA; #xA;', $rest) / + /xsl:otherwise +/xsl:choose + /xsl:with-param + xsl:with-param name=rest select=substring-after($rest, '#xA;#xA;')/ +/xsl:call-template + /xsl:when + xsl:when test=starts-with($block, ' ') +pre class=codexsl:for-each select=str:tokenize($block, '#xA;') + xsl:choose +xsl:when test=starts-with(., ' ') + xsl:value-of select=substring(., 3)/ +/xsl:when +xsl:otherwise + xsl:value-of select=./ +/xsl:otherwise + /xsl:choose + xsl:if test=position() != last() +xsl:text#xA;/xsl:text + /xsl:if +/xsl:for-each/pre + /xsl:when + xsl:otherwise +p + xsl:call-template name=dumptext +xsl:with-param name=text select=$block/ + /xsl:call-template +/p + /xsl:otherwise +/xsl:choose +xsl:if test=not($multipleCodeBlocks) + xsl:call-template name=formattext +xsl:with-param name=text select=$rest/ + /xsl:call-template +/xsl:if + /xsl:template + xsl:template name=formattext xsl:param name=text / @@ -157,28 +218,19 @@ xsl:variable name=head select=substring-before($text, '#xA;#xA;')/ xsl:variable name=rest select=substring-after($text, '#xA;#xA;')/ - xsl:choose -xsl:when test=$head - p -xsl:call-template name=dumptext - xsl:with-param name=text select=$head/ -/xsl:call-template - /p -/xsl:when -xsl:when test=not($rest) - p -xsl:call-template name=dumptext - xsl:with-param name=text select=$text/ -/xsl:call-template - /p -/xsl:when - /xsl:choose - - xsl:if test=$rest -xsl:call-template name=formattext - xsl:with-param name=text select=$rest/ -/xsl:call-template - /xsl:if + xsl:call-template name=formatblock +xsl:with-param name=block + xsl:choose +xsl:when test=contains($text, '#xA;#xA;') + xsl:value-of select=$head/ +/xsl:when +xsl:otherwise + xsl:value-of select=$text/ +/xsl:otherwise + /xsl:choose +/xsl:with-param +xsl:with-param name=rest select=$rest/ + /xsl:call-template /xsl:if /xsl:template -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] libvirt.c: indent code of virDomainGetMemoryParameters's documentation
By indenting code inside of comments, it gets recognized as a
code block when generating the HTML documentation.
Signed-off-by: Claudio Bley cb...@av-test.de
---
src/libvirt.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 405c13f..c48bc7b 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -4057,14 +4057,14 @@ error:
*
* Here is a sample code snippet:
*
- * if ((virDomainGetMemoryParameters(dom, NULL, nparams, 0) == 0)
- * (nparams != 0)) {
- * if ((params = malloc(sizeof(*params) * nparams)) == NULL)
- * goto error;
- * memset(params, 0, sizeof(*params) * nparams);
- * if (virDomainGetMemoryParameters(dom, params, nparams, 0))
- * goto error;
- * }
+ * if ((virDomainGetMemoryParameters(dom, NULL, nparams, 0) == 0)
+ * (nparams != 0)) {
+ * if ((params = malloc(sizeof(*params) * nparams)) == NULL)
+ * goto error;
+ * memset(params, 0, sizeof(*params) * nparams);
+ * if (virDomainGetMemoryParameters(dom, params, nparams, 0))
+ * goto error;
+ * }
*
* This function may require privileged access to the hypervisor. This function
* expects the caller to allocate the @params.
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] libvirt.c: add 2 spaces of indentation to example code of virStreamSend
See libvirt-libvirt.html#virStreamSend for the effect.
Signed-off-by: Claudio Bley cb...@av-test.de
---
src/libvirt.c | 114 -
1 file changed, 57 insertions(+), 57 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 9f579a6..405c13f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -16992,37 +16992,37 @@ virStreamRef(virStreamPtr stream)
* An example using this with a hypothetical file upload
* API looks like
*
- * virStreamPtr st = virStreamNew(conn, 0);
- * int fd = open(demo.iso, O_RDONLY)
+ * virStreamPtr st = virStreamNew(conn, 0);
+ * int fd = open(demo.iso, O_RDONLY)
*
- * virConnectUploadFile(conn, demo.iso, st);
- *
- * while (1) {
- * char buf[1024];
- * int got = read(fd, buf, 1024);
- * if (got 0) {
- * virStreamAbort(st);
- * break;
- * }
- * if (got == 0) {
- * virStreamFinish(st);
- * break;
- * }
- * int offset = 0;
- * while (offset got) {
- * int sent = virStreamSend(st, buf+offset, got-offset)
- * if (sent 0) {
+ * virConnectUploadFile(conn, demo.iso, st);
+ *
+ * while (1) {
+ * char buf[1024];
+ * int got = read(fd, buf, 1024);
+ * if (got 0) {
* virStreamAbort(st);
- * goto done;
+ * break;
* }
- * offset += sent;
- * }
- * }
- * if (virStreamFinish(st) 0)
- * ... report an error
- * done:
- * virStreamFree(st);
- * close(fd);
+ * if (got == 0) {
+ * virStreamFinish(st);
+ * break;
+ * }
+ * int offset = 0;
+ * while (offset got) {
+ * int sent = virStreamSend(st, buf+offset, got-offset)
+ * if (sent 0) {
+ *virStreamAbort(st);
+ *goto done;
+ * }
+ * offset += sent;
+ * }
+ * }
+ * if (virStreamFinish(st) 0)
+ * ... report an error
+ *done:
+ * virStreamFree(st);
+ * close(fd);
*
* Returns the number of bytes written, which may be less
* than requested.
@@ -17086,35 +17086,35 @@ error:
* An example using this with a hypothetical file download
* API looks like
*
- * virStreamPtr st = virStreamNew(conn, 0);
- * int fd = open(demo.iso, O_WRONLY, 0600)
- *
- * virConnectDownloadFile(conn, demo.iso, st);
- *
- * while (1) {
- * char buf[1024];
- * int got = virStreamRecv(st, buf, 1024);
- * if (got 0)
- * break;
- * if (got == 0) {
- * virStreamFinish(st);
- * break;
- * }
- * int offset = 0;
- * while (offset got) {
- * int sent = write(fd, buf+offset, got-offset)
- * if (sent 0) {
- * virStreamAbort(st);
- * goto done;
- * }
- * offset += sent;
- * }
- * }
- * if (virStreamFinish(st) 0)
- * ... report an error
- * done:
- * virStreamFree(st);
- * close(fd);
+ * virStreamPtr st = virStreamNew(conn, 0);
+ * int fd = open(demo.iso, O_WRONLY, 0600)
+ *
+ * virConnectDownloadFile(conn, demo.iso, st);
+ *
+ * while (1) {
+ * char buf[1024];
+ * int got = virStreamRecv(st, buf, 1024);
+ * if (got 0)
+ *break;
+ * if (got == 0) {
+ *virStreamFinish(st);
+ *break;
+ * }
+ * int offset = 0;
+ * while (offset got) {
+ *int sent = write(fd, buf+offset, got-offset)
+ *if (sent 0) {
+ * virStreamAbort(st);
+ * goto done;
+ *}
+ *offset += sent;
+ * }
+ * }
+ * if (virStreamFinish(st) 0)
+ *... report an error
+ * done:
+ * virStreamFree(st);
+ * close(fd);
*
*
* Returns the number of bytes read, which may be less
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/4] don't masquerade local broadcast/multicast packets
On 09/24/2013 02:38 PM, Laszlo Ersek wrote: I might as well fix up v2 as requested originally, and submit that as v4. What do you recommend? I think fixing up v2 with the renames is a better approach. I'm fine either way, I'd just like to get this merged and stop wasting the time of y'all. That sounds fine to me. Again, sorry for causing much of the unnecessary churn. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH]util: Refresh virHook before checking its existence
From: Chen Hanxiao chenhanx...@cn.fujitsu.com
We refresh the status of hook scripts
only when start/stop libvirt, or reload its configuration.
But the status of hooks scripts may be changed.
We need to refresh its status before checking its existence.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/util/virhook.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/util/virhook.c b/src/util/virhook.c
index 159efdb..5500f62 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -170,11 +170,15 @@ virHookPresent(int driver) {
if ((driver VIR_HOOK_DRIVER_DAEMON) ||
(driver = VIR_HOOK_DRIVER_LAST))
return 0;
-if (virHooksFound == -1)
-return 0;
+if (virHookInitialize() 0) {
+if (virHooksFound == -1)
+return 0;
-if ((virHooksFound (1 driver)) == 0)
+if ((virHooksFound (1 driver)) == 0)
+return 0;
+} else {
return 0;
+}
return 1;
}
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/4] don't masquerade local broadcast/multicast packets
On 09/25/13 09:22, Laine Stump wrote: On 09/24/2013 02:38 PM, Laszlo Ersek wrote: I might as well fix up v2 as requested originally, and submit that as v4. What do you recommend? I think fixing up v2 with the renames is a better approach. I'm fine either way, I'd just like to get this merged and stop wasting the time of y'all. That sounds fine to me. Again, sorry for causing much of the unnecessary churn. No problem. Thanks! Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH]util: Refresh virHook before checking its existence
On Wed, Sep 25, 2013 at 03:48:16PM +0800, Chen Hanxiao wrote:
From: Chen Hanxiao chenhanx...@cn.fujitsu.com
We refresh the status of hook scripts
only when start/stop libvirt, or reload its configuration.
But the status of hooks scripts may be changed.
We need to refresh its status before checking its existence.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/util/virhook.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/util/virhook.c b/src/util/virhook.c
index 159efdb..5500f62 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -170,11 +170,15 @@ virHookPresent(int driver) {
if ((driver VIR_HOOK_DRIVER_DAEMON) ||
(driver = VIR_HOOK_DRIVER_LAST))
return 0;
-if (virHooksFound == -1)
-return 0;
+if (virHookInitialize() 0) {
+if (virHooksFound == -1)
+return 0;
-if ((virHooksFound (1 driver)) == 0)
+if ((virHooksFound (1 driver)) == 0)
+return 0;
+} else {
return 0;
+}
return 1;
}
Doing this is not thread safe. This can be called from multiple threads
at once.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/17] Fix leak in qemuStringToArgvEnv upon OOM
On 09/24/2013 06:04 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The 'qemuStringToArgvEnv' method splits up a string of command
line env/args to an 'arglist' array. It then copies env vars
to a 'progenv' array and args to a 'progargv' array. When
copyin the env vars, it NULL-ifies the element in 'arglist'
that is copied.
Upon OOM the 'virStringListFree' is called on progenv and
arglist. Unfortunately, because the elements in 'arglist'
related to env vars have been set to NULL, the call to
virStringListFree(arglist) doesn't free anything, even
though some non-NULL args vars still exist later in the
array.
To fix this leak, stop NULL-ifying the 'arglist' elements,
and change the cleanup code to only free elements in the
'arglist' array, not 'progenv'.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_command.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fe53cf7..7029335 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9724,10 +9724,8 @@ static int qemuStringToArgvEnv(const char *args,
if (envend 0) {
if (VIR_REALLOC_N(progenv, envend+1) 0)
goto error;
-for (i = 0; i envend; i++) {
+for (i = 0; i envend; i++)
progenv[i] = arglist[i];
-arglist[i] = NULL;
-}
progenv[i] = NULL;
}
@@ -9746,7 +9744,8 @@ static int qemuStringToArgvEnv(const char *args,
return 0;
error:
-virStringFreeList(progenv);
+VIR_FREE(progenv);
+VIR_FREE(progargv);
We don't jump to error after progargv is allocated, but it doesn't hurt.
virStringFreeList(arglist);
return -1;
}
ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/17] Fix leak in qemuParseCommandLine on OOM
On 09/24/2013 06:04 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com If the call to virDomainControllerInsert fails in qemuParseCommandLine, the controller struct is leaked. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_command.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/17] Fix leak in qemuParseCommandLineDisk on OOM
On Tue, Sep 24, 2013 at 04:24:43PM -0500, Doug Goldstein wrote:
On Tue, Sep 24, 2013 at 11:03 AM, Daniel P. Berrange
berra...@redhat.com wrote:
From: Daniel P. Berrange berra...@redhat.com
If OOM occurs in qemuParseCommandLineDisk some intermediate
variables will be leaked when parsing Sheepdog or RBD disks.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_command.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a82c5dd..f6a3aa2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9935,8 +9935,10 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr
xmlopt,
if (VIR_STRDUP(def-src, p + strlen(rbd:)) 0)
goto error;
/* old-style CEPH_ARGS env variable is parsed later */
-if (!old_style_ceph_args qemuParseRBDString(def)
0)
-goto cleanup;
+if (!old_style_ceph_args qemuParseRBDString(def)
0) {
+VIR_FREE(p);
+goto error;
+}
VIR_FREE(p);
} else if (STRPREFIX(def-src, gluster:) ||
@@ -9960,17 +9962,20 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr
xmlopt,
def-protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG;
if (VIR_STRDUP(def-src, p + strlen(sheepdog:)) 0)
goto error;
+VIR_FREE(p);
/* def-src must be [vdiname] or
[host]:[port]:[vdiname] */
port = strchr(def-src, ':');
if (port) {
-*port++ = '\0';
-vdi = strchr(port, ':');
+*port = '\0';
+vdi = strchr(port + 1, ':');
if (!vdi) {
+*port = ':';
Is this bit necessary? Or is it for making the error message look better?
Yep, we want to show the user their original full input, not the truncated
one.
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot parse sheepdog
filename '%s'), p);
+ _(cannot parse sheepdog
filename '%s'), def-src);
goto error;
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/4] iptablesFormatNetwork(): constify target of netaddr parameter
On 09/24/2013 09:03 AM, Laszlo Ersek wrote: On 09/24/13 10:46, Laine Stump wrote: On 09/23/2013 08:03 PM, Laszlo Ersek wrote: ... and adapt functions that would cast away the new const qualifier. Given typedef virSocketAddr *virSocketAddrPtr; Compare the parse trees of the following two declarations: (a) const virSocketAddrPtr addr; (b) const virSocketAddr*addr; Umm.. Eric? A little help? :-) The grammar rules that I used for the AST derivation can be looked up eg. in the final C11 draft, http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf Section 6.7 Declarations. But, the short version is really just that type qualifiers (like const volatile) don't enter the typedef name; they qualify the variable being declared. I like your explanation better :-) In general I disapprove of typedefs: they seem to be friendly by saving you the repeated typing of struct and *. Until they trick you :) I like typedefs for eliminating repetitive typing of struct, but not for removing * - that seems pointless to me (pun not intended), since you're not saving any characters, and losing track of the nice * that everyone is used to seeing. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/17] Fix leak of char device in xenParseXM
On 09/24/2013 06:04 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com If an OOM occurs in xenParseXM, a virDomainChrDef may be leaked. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/xenxs/xen_xm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/17] Fix leak of command line args in qemuParseCommandLine
On 09/24/2013 06:04 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com If qemuParseCommandLine finds an arg it does not understand it adds it to the QEMU passthrough custom arg list. If the qemuParseCommandLine method hits an error for any reason though, it just does 'VIR_FREE(cmd)' on the custom arg list. This means all actual args / env vars are leaked. Introduce a qemuDomainCmdlineDefFree method to be used for cleanup. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_conf.c| 18 ++ src/qemu/qemu_conf.h| 2 ++ src/qemu/qemu_domain.c | 15 +-- 4 files changed, 23 insertions(+), 16 deletions(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/17] Fix crash on OOM in xenParseXM handling consoles
On 09/24/2013 06:04 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The xenParseXM sets def-nconsoles to 1 before claling VIR_REALLOC_N on def-consoles. So if the alloc fails due to OOM, the cleanup code will crash accessing a console that does not exist. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/xenxs/xen_xm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 17/17] Fix leak of serial value in xenFormatXM on OOM
On 09/24/2013 06:04 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com If an OOM occurs in xenFormatXM when formatting to the serial device value, the value is leaked. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/xenxs/xen_xm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 16/17] Fix broken formatting on OOM in xenFormatXM
On 09/24/2013 06:04 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com If an OOM occurs when xenFormatXM is setting the 'hpet' variable it is silently ignored. Fix it to propagate to the callers. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/xenxs/xen_xm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] Attaching PCI devices to the PCIe root complex
On Wed, Sep 25, 2013 at 11:48:28AM +0300, Marcel Apfelbaum wrote: On Wed, 2013-09-25 at 10:01 +0300, Michael S. Tsirkin wrote: On Tue, Sep 24, 2013 at 06:01:02AM -0400, Laine Stump wrote: When I added support for the Q35-based machinetypes to libvirt, I specifically prohibited attaching any PCI devices (with the exception of graphics controllers) to the PCIe root complex, That's wrong I think. Anything attached to RC is an integrated endpoint, and these can be PCI devices. I couldn't find on PCIe spec any mention that Root Complex Integrated EndPoint must be PCIe. But, from spec 1.3.2.3: - A Root Complex Integrated Endpoint must not require I/O resources claimed through BAR(s). - A Root Complex Integrated Endpoint must not generate I/O Requests. - A Root Complex Integrated Endpoint is required to support MSI or MSI-X or both if an interrupt resource is requested. Heh PCI-SIG keeps fighting against legacy interrupts and IO. But lots of hardware happily ignores these rules. And the reason is simple: software does not enforce them. Here's integrated stuff on my laptop: 00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09) (prog-if 00 [VGA controller]) Subsystem: Lenovo Device 21cf Flags: bus master, fast devsel, latency 0, IRQ 43 Memory at f000 (64-bit, non-prefetchable) [size=4M] Memory at e000 (64-bit, prefetchable) [size=256M] I/O ports at 5000 [size=64] Expansion ROM at unassigned [disabled] Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit- Capabilities: [d0] Power Management version 2 Capabilities: [a4] PCI Advanced Features Kernel driver in use: i915 So it has an IO BAR. 00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #2 (rev 04) (prog-if 20 [EHCI]) Subsystem: Lenovo Device 21cf Flags: bus master, medium devsel, latency 0, IRQ 16 Memory at f252a000 (32-bit, non-prefetchable) [size=1K] Capabilities: [50] Power Management version 2 Capabilities: [58] Debug port: BAR=1 offset=00a0 Capabilities: [98] PCI Advanced Features Kernel driver in use: ehci-pci So IRQ but no MSI. I suppose that this restriction can be removed for PCI devices that 1. Actually work when plugged in into RC Integrated EndPoint 2. Respond to the above limitations These limitations are just guidance for future devices. They can't change the past, such devices were made. and had planned to prevent attaching them to PCIe root ports (ioh3420 device) and PCIe downstream switch ports (xio-3130 device) as well. I did this because, even though qemu currently allows attaching a normal PCI device in any of these three places, the restriction exists for real hardware and I didn't see any guarantee that qemu wouldn't add the restriction in the future in order to more closely emulate real hardware. However, since I did that, I've learned that many of the qemu pci devices really should be considered as pci or pcie. Gerd Hoffman lists some of these cases in a bug he filed against libvirt: https://bugzilla.redhat.com/show_bug.cgi?id=1003983 I would like to loosen up the restrictions in libvirt, but want to make sure that I don't allow something that could later be forbidden by qemu (thus creating a compatibility problem during upgrades). Beyond Gerd's specific requests to allow ehci, uhci, and hda controllers to attach to PCIe ports, are there any other devices that I specifically should or shouldn't allow? (I would rather be conservative in what I allow - it's easy to allow more things later, but nearly impossible to revoke permission once it's been allowed). For the moment I would not remove any restrictions, but only the ones requested and verified by somebody. IMO, we really need to grow an interface to query this kind of thing. Basically libvirt needs to know: 1. for (libvirt) controllers: what kind of devices can be plugged in 2. for devices (controller is also a device) - to which controllers can it be plugged in - does it support hot-plug? 3. implicit controllers of the machine types (q35 - pcie-root, i440fx - pci-root) All the above must be exported to libvirt Implementation options: 1. Add a compliance field on PCI/PCIe devices and controllers stating if it supports PCI/PCIe or both (and maybe hot-plug) - consider plug type + compliance to figure out whether a plug can go into a socket 2. Use Markus Armbruster idea of introducing a concept of plug and sockets: - dividing the devices into adapters and plugs - adding sockets to bridges(buses?). In this way it would be clear which devices can connect to bridges Any thoughts? Thanks, Marcel It's all not too hard to implement, we
[libvirt] [PATCH]util: Helper function for checking existence of hook files for specific driver
From: Chen Hanxiao chenhanx...@cn.fujitsu.com
We refresh the status of hook scripts
only when start/restart libvirt or reloads its configuration.
But hooks scripts may be changed.
This function will help to check its existence.
And we do not need to start/restart libvirt if
we add/remove hook files.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/util/virhook.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/util/virhook.c b/src/util/virhook.c
index 159efdb..c4a1a15 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -129,6 +129,12 @@ virHookCheck(int no, const char *driver) {
return ret;
}
+static int
+virHookDriverCheck(int driver) {
+return virHookCheck(driver,
+virHookDriverTypeToString(driver));
+}
+
/*
* virHookInitialize:
*
@@ -170,11 +176,12 @@ virHookPresent(int driver) {
if ((driver VIR_HOOK_DRIVER_DAEMON) ||
(driver = VIR_HOOK_DRIVER_LAST))
return 0;
-if (virHooksFound == -1)
+if (virHookDriverCheck(driver) != 1) {
+VIR_DEBUG(Driver %s hooks files not found,
+ virHookDriverTypeToString(driver));
return 0;
+}
-if ((virHooksFound (1 driver)) == 0)
-return 0;
return 1;
}
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH]util: Helper function for checking existence of hook files for specific driver
On Wed, Sep 25, 2013 at 05:50:47PM +0800, Chen Hanxiao wrote:
From: Chen Hanxiao chenhanx...@cn.fujitsu.com
We refresh the status of hook scripts
only when start/restart libvirt or reloads its configuration.
But hooks scripts may be changed.
This function will help to check its existence.
And we do not need to start/restart libvirt if
we add/remove hook files.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/util/virhook.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/util/virhook.c b/src/util/virhook.c
index 159efdb..c4a1a15 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -129,6 +129,12 @@ virHookCheck(int no, const char *driver) {
return ret;
}
+static int
+virHookDriverCheck(int driver) {
+return virHookCheck(driver,
+virHookDriverTypeToString(driver));
+}
+
/*
* virHookInitialize:
*
@@ -170,11 +176,12 @@ virHookPresent(int driver) {
if ((driver VIR_HOOK_DRIVER_DAEMON) ||
(driver = VIR_HOOK_DRIVER_LAST))
return 0;
-if (virHooksFound == -1)
+if (virHookDriverCheck(driver) != 1) {
+VIR_DEBUG(Driver %s hooks files not found,
+ virHookDriverTypeToString(driver));
return 0;
+}
-if ((virHooksFound (1 driver)) == 0)
-return 0;
return 1;
}
This is no more thread safe than your previous patch.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 1/2] util/viriptables: add/remove rules that short-circuit masquerading
The functions
- iptablesAddForwardDontMasquerade(),
- iptablesRemoveForwardDontMasquerade
handle exceptions in the masquerading implemented in the POSTROUTING chain
of the nat table. Such exceptions should be added as chronologically
latest, logically top-most rules.
The bridge driver will call these functions beginning with the next patch:
some special destination IP addresses always refer to the local
subnetwork, even though they don't match any practical subnetwork's
netmask. Packets from virbrN targeting such IP addresses are never routed
outwards, but the current rules treat them as non-virbrN-destined packets
and masquerade them. This causes problems for some receivers on virbrN.
Signed-off-by: Laszlo Ersek ler...@redhat.com
---
v2-v4:
- Rename iptables(Add|Remove)ForwardDontMasquerade to
iptables(Add|Remove)DontMasquerade [Laine].
src/util/viriptables.h | 8 +
src/util/viriptables.c | 88
src/libvirt_private.syms | 2 ++
3 files changed, 98 insertions(+)
diff --git a/src/util/viriptables.h b/src/util/viriptables.h
index 447f4a8..6dfb992 100644
--- a/src/util/viriptables.h
+++ b/src/util/viriptables.h
@@ -94,6 +94,14 @@ int iptablesRemoveForwardMasquerade
(virSocketAddr *netaddr,
virSocketAddrRangePtr addr,
virPortRangePtr port,
const char *protocol);
+int iptablesAddDontMasquerade (virSocketAddr *netaddr,
+ unsigned int prefix,
+ const char *physdev,
+ const char *destaddr);
+int iptablesRemoveDontMasquerade(virSocketAddr *netaddr,
+ unsigned int prefix,
+ const char *physdev,
+ const char *destaddr);
int iptablesAddOutputFixUdpChecksum (const char *iface,
int port);
int iptablesRemoveOutputFixUdpChecksum (const char *iface,
diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index 0f57d66..16f571e 100644
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -832,6 +832,94 @@ iptablesRemoveForwardMasquerade(virSocketAddr *netaddr,
}
+/* Don't masquerade traffic coming from the network associated with the bridge
+ * if said traffic targets @destaddr.
+ */
+static int
+iptablesForwardDontMasquerade(virSocketAddr *netaddr,
+ unsigned int prefix,
+ const char *physdev,
+ const char *destaddr,
+ int action)
+{
+int ret = -1;
+char *networkstr = NULL;
+virCommandPtr cmd = NULL;
+
+if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
+return -1;
+
+if (!VIR_SOCKET_ADDR_IS_FAMILY(netaddr, AF_INET)) {
+/* Higher level code *should* guaranteee it's impossible to get here.
*/
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Attempted to NAT '%s'. NAT is only supported for
IPv4.),
+ networkstr);
+goto cleanup;
+}
+
+cmd = iptablesCommandNew(nat, POSTROUTING, AF_INET, action);
+
+if (physdev physdev[0])
+virCommandAddArgList(cmd, --out-interface, physdev, NULL);
+
+virCommandAddArgList(cmd, --source, networkstr,
+ --destination, destaddr, --jump, RETURN, NULL);
+ret = virCommandRun(cmd, NULL);
+cleanup:
+virCommandFree(cmd);
+VIR_FREE(networkstr);
+return ret;
+}
+
+/**
+ * iptablesAddDontMasquerade:
+ * @netaddr: the source network name
+ * @prefix: prefix (# of 1 bits) of netmask to apply to @netaddr
+ * @physdev: the physical output device or NULL
+ * @destaddr: the destination network not to masquerade for
+ *
+ * Add rules to the IP table context to avoid masquerading from
+ * @netaddr/@prefix to @destaddr on @physdev. @destaddr must be in a format
+ * directly consumable by iptables, it must not depend on user input or
+ * configuration.
+ *
+ * Returns 0 in case of success or an error code otherwise.
+ */
+int
+iptablesAddDontMasquerade(virSocketAddr *netaddr,
+ unsigned int prefix,
+ const char *physdev,
+ const char *destaddr)
+{
+return iptablesForwardDontMasquerade(netaddr, prefix, physdev, destaddr,
+ ADD);
+}
+
+/**
+ * iptablesRemoveDontMasquerade:
+ * @netaddr: the source network name
+ * @prefix: prefix (# of 1 bits) of netmask to apply to @netaddr
+ * @physdev: the physical output device or NULL
+ * @destaddr: the destination network not to
[libvirt] [PATCH v4 0/2] don't masquerade local broadcast/multicast packets
v2-v4 changes (v3 went in a different direction): - Rename iptables(Add|Remove)ForwardDontMasquerade to iptables(Add|Remove)DontMasquerade [Laine]. Masquerading local broadcast breaks DHCP replies for some clients. There has been a report about broken local multicast too. (See references in the patches.) Testing: Chain POSTROUTING (policy ACCEPT 2 packets, 134 bytes) pkts bytes target prot opt in out source destination 00 RETURN all -- * * 192.168.122.0/24 224.0.0.0/24 00 RETURN all -- * * 192.168.122.0/24 255.255.255.255 00 MASQUERADE tcp -- * * 192.168.122.0/24 !192.168.122.0/24masq ports: 1024-65535 00 MASQUERADE udp -- * * 192.168.122.0/24 !192.168.122.0/24masq ports: 1024-65535 00 MASQUERADE all -- * * 192.168.122.0/24 !192.168.122.0/24 + make check, make syntax-check, virsh net-start / net-destroy. Laszlo Ersek (2): util/viriptables: add/remove rules that short-circuit masquerading bridge driver: don't masquerade local subnet broadcast/multicast packets src/util/viriptables.h| 8 src/network/bridge_driver_linux.c | 70 +-- src/util/viriptables.c| 88 +++ src/libvirt_private.syms | 2 + 4 files changed, 164 insertions(+), 4 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 2/2] bridge driver: don't masquerade local subnet broadcast/multicast packets
Packets sent by guests on virbrN, *or* by dnsmasq on the same, to
- 255.255.255.255/32 (netmask-independent local network broadcast
address), or to
- 224.0.0.0/24 (local subnetwork multicast range)
are never forwarded, hence it is not necessary to masquerade them.
In fact we must not masquerade them: translating their source addresses or
source ports (where applicable) may confuse receivers on virbrN.
One example is the DHCP client in OVMF (= UEFI firmware for virtual
machines):
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/1506/focus=2640
It expects DHCP replies to arrive from remote source port 67. Even though
dnsmasq conforms to that, the destination address (255.255.255.255) and
the source address (eg. 192.168.122.1) in the reply allow the UDP
masquerading rule to match, which rewrites the source port to or above
1024. This prevents the DHCP client in OVMF from accepting the packet.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=709418
Signed-off-by: Laszlo Ersek ler...@redhat.com
---
v2-v4:
- Rename iptables(Add|Remove)ForwardDontMasquerade to
iptables(Add|Remove)DontMasquerade [Laine].
src/network/bridge_driver_linux.c | 70 ---
1 file changed, 66 insertions(+), 4 deletions(-)
diff --git a/src/network/bridge_driver_linux.c
b/src/network/bridge_driver_linux.c
index 80430a8..066779a 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -127,6 +127,9 @@ out:
return ret;
}
+static const char networkLocalMulticast[] = 224.0.0.0/24;
+static const char networkLocalBroadcast[] = 255.255.255.255/32;
+
int networkAddMasqueradingFirewallRules(virNetworkObjPtr network,
virNetworkIpDefPtr ipdef)
{
@@ -167,11 +170,20 @@ int networkAddMasqueradingFirewallRules(virNetworkObjPtr
network,
/*
* Enable masquerading.
*
- * We need to end up with 3 rules in the table in this order
+ * We need to end up with 5 rules in the table in this order
*
- * 1. protocol=tcp with sport mapping restriction
- * 2. protocol=udp with sport mapping restriction
- * 3. generic any protocol
+ * 1. do not masquerade packets targeting 224.0.0.0/24
+ * 2. do not masquerade packets targeting 255.255.255.255/32
+ * 3. masquerade protocol=tcp with sport mapping restriction
+ * 4. masquerade protocol=udp with sport mapping restriction
+ * 5. generic, masquerade any protocol
+ *
+ * 224.0.0.0/24 is the local network multicast range. Packets are not
+ * forwarded outside.
+ *
+ * 255.255.255.255/32 is the broadcast address of any local network. Again,
+ * such packets are never forwarded, but strict DHCP clients don't accept
+ * DHCP replies with changed source ports.
*
* The sport mappings are required, because default IPtables
* MASQUERADE maintain port numbers unchanged where possible.
@@ -238,8 +250,50 @@ int networkAddMasqueradingFirewallRules(virNetworkObjPtr
network,
goto masqerr5;
}
+/* exempt local network broadcast address as destination */
+if (iptablesAddDontMasquerade(ipdef-address,
+ prefix,
+ forwardIf,
+ networkLocalBroadcast) 0) {
+if (forwardIf)
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _(failed to add iptables rule to prevent local
broadcast masquerading on %s),
+ forwardIf);
+else
+virReportError(VIR_ERR_SYSTEM_ERROR, %s,
+ _(failed to add iptables rule to prevent local
broadcast masquerading));
+goto masqerr6;
+}
+
+/* exempt local multicast range as destination */
+if (iptablesAddDontMasquerade(ipdef-address,
+ prefix,
+ forwardIf,
+ networkLocalMulticast) 0) {
+if (forwardIf)
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _(failed to add iptables rule to prevent local
multicast masquerading on %s),
+ forwardIf);
+else
+virReportError(VIR_ERR_SYSTEM_ERROR, %s,
+ _(failed to add iptables rule to prevent local
multicast masquerading));
+goto masqerr7;
+}
+
return 0;
+ masqerr7:
+iptablesRemoveDontMasquerade(ipdef-address,
+ prefix,
+ forwardIf,
+ networkLocalBroadcast);
+ masqerr6:
+iptablesRemoveForwardMasquerade(ipdef-address,
+prefix,
+forwardIf,
+network-def-forward.addr,
+network-def-forward.port,
+
Re: [libvirt] [PATCHv2 0/4] Improve passthrough of early errors from qemu log
On 09/24/2013 05:14 PM, Peter Krempa wrote: Version 2 fixed according to review feedback of Jan. Peter Krempa (4): qemu_process: Make qemuProcessReadLog() more versatile and reusable qemu: monitor: Add infrastructure to access VM logs for better err msgs qemu: monitor: Produce better errors on monitor hangup qemu: Wire up better early error reporting src/qemu/qemu_monitor.c | 93 + src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_process.c | 54 +++- src/qemu/qemu_process.h | 2 ++ 4 files changed, 129 insertions(+), 22 deletions(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] LXC: Detect fs support. Mount only supported fs in containers
Some filesystems - specifically securityfs - are not supported in
all systems running libvirt containers. When starting a container,
mount only the filesystems that are supported on the host. Detection
of filesystem support is done at runtime.
Signed-off-by: Bogdan Purcareata bogdan.purcare...@freescale.com
---
src/lxc/lxc_container.c | 61 +
1 file changed, 61 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index c60f5d8..eff9a24 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -509,6 +509,61 @@ static int lxcContainerChildMountSort(const void *a, const
void *b)
# define MS_SLAVE(119)
#endif
+/*
+ * This function attempts to detect kernel support
+ * for a specific filesystem type. This is done by
+ * inspecting /proc/filesystems.
+ */
+static int lxcCheckFSSupport(const char *fs_type)
+{
+FILE *fp = NULL;
+int ret = -1;
+const char *fslist = /proc/filesystems;
+char *line = NULL;
+const char *type;
+
+if (!fs_type)
+return 1;
+
+VIR_DEBUG(Checking kernel support for %s, fs_type);
+
+VIR_DEBUG(Open %s, fslist);
+if (!(fp = fopen(fslist, r))) {
+if (errno == ENOENT)
+
+virReportSystemError(errno,
+ _(Unable to read %s),
+ fslist);
+goto cleanup;
+}
+
+while (!feof(fp)) {
+size_t n;
+VIR_FREE(line);
+if (getline(line, n, fp) = 0) {
+if (feof(fp))
+break;
+
+goto cleanup;
+}
+
+type = strstr(line, fs_type);
+if (type) {
+ret = 1;
+goto cleanup;
+}
+}
+
+VIR_DEBUG(No kernel support for %s, fs_type);
+
+ret = 0;
+
+cleanup:
+VIR_FREE(line);
+VIR_FORCE_FCLOSE(fp);
+return ret;
+}
+
static int lxcContainerGetSubtree(const char *prefix,
char ***mountsret,
size_t *nmountsret)
@@ -855,17 +910,23 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
for (i = 0; i ARRAY_CARDINALITY(lxcBasicMounts); i++) {
virLXCBasicMountInfo const *mnt = lxcBasicMounts[i];
const char *srcpath = NULL;
+ const char *dstpath = NULL;
VIR_DEBUG(Processing %s - %s,
mnt-src, mnt-dst);
srcpath = mnt-src;
+ dstpath = mnt-dst;
/* Skip if mount doesn't exist in source */
if ((srcpath[0] == '/')
(access(srcpath, R_OK) 0))
continue;
+ if ((access(dstpath, R_OK) 0) || /* mount is not present on host */
+ (!lxcCheckFSSupport(mnt-type))) /* no fs support in kernel */
+ continue;
+
#if WITH_SELINUX
if (STREQ(mnt-src, SELINUX_MOUNT)
(!is_selinux_enabled() || userns_enabled))
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/2] util/viriptables: add/remove rules that short-circuit masquerading
On 09/25/2013 06:45 AM, Laszlo Ersek wrote:
The functions
- iptablesAddForwardDontMasquerade(),
- iptablesRemoveForwardDontMasquerade
handle exceptions in the masquerading implemented in the POSTROUTING chain
of the nat table. Such exceptions should be added as chronologically
latest, logically top-most rules.
The bridge driver will call these functions beginning with the next patch:
some special destination IP addresses always refer to the local
subnetwork, even though they don't match any practical subnetwork's
netmask. Packets from virbrN targeting such IP addresses are never routed
outwards, but the current rules treat them as non-virbrN-destined packets
and masquerade them. This causes problems for some receivers on virbrN.
ACK, with the small change to to note the new function names in the
commit log message as well. I'll fix that before I push it.
Signed-off-by: Laszlo Ersek ler...@redhat.com
---
v2-v4:
- Rename iptables(Add|Remove)ForwardDontMasquerade to
iptables(Add|Remove)DontMasquerade [Laine].
src/util/viriptables.h | 8 +
src/util/viriptables.c | 88
src/libvirt_private.syms | 2 ++
3 files changed, 98 insertions(+)
diff --git a/src/util/viriptables.h b/src/util/viriptables.h
index 447f4a8..6dfb992 100644
--- a/src/util/viriptables.h
+++ b/src/util/viriptables.h
@@ -94,6 +94,14 @@ int iptablesRemoveForwardMasquerade
(virSocketAddr *netaddr,
virSocketAddrRangePtr addr,
virPortRangePtr port,
const char *protocol);
+int iptablesAddDontMasquerade (virSocketAddr *netaddr,
+ unsigned int prefix,
+ const char *physdev,
+ const char *destaddr);
+int iptablesRemoveDontMasquerade(virSocketAddr *netaddr,
+ unsigned int prefix,
+ const char *physdev,
+ const char *destaddr);
int iptablesAddOutputFixUdpChecksum (const char *iface,
int port);
int iptablesRemoveOutputFixUdpChecksum (const char *iface,
diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index 0f57d66..16f571e 100644
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -832,6 +832,94 @@ iptablesRemoveForwardMasquerade(virSocketAddr *netaddr,
}
+/* Don't masquerade traffic coming from the network associated with the
bridge
+ * if said traffic targets @destaddr.
+ */
+static int
+iptablesForwardDontMasquerade(virSocketAddr *netaddr,
+ unsigned int prefix,
+ const char *physdev,
+ const char *destaddr,
+ int action)
+{
+int ret = -1;
+char *networkstr = NULL;
+virCommandPtr cmd = NULL;
+
+if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
+return -1;
+
+if (!VIR_SOCKET_ADDR_IS_FAMILY(netaddr, AF_INET)) {
+/* Higher level code *should* guaranteee it's impossible to get
here. */
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Attempted to NAT '%s'. NAT is only supported for
IPv4.),
+ networkstr);
+goto cleanup;
+}
+
+cmd = iptablesCommandNew(nat, POSTROUTING, AF_INET, action);
+
+if (physdev physdev[0])
+virCommandAddArgList(cmd, --out-interface, physdev, NULL);
+
+virCommandAddArgList(cmd, --source, networkstr,
+ --destination, destaddr, --jump, RETURN,
NULL);
+ret = virCommandRun(cmd, NULL);
+cleanup:
+virCommandFree(cmd);
+VIR_FREE(networkstr);
+return ret;
+}
+
+/**
+ * iptablesAddDontMasquerade:
+ * @netaddr: the source network name
+ * @prefix: prefix (# of 1 bits) of netmask to apply to @netaddr
+ * @physdev: the physical output device or NULL
+ * @destaddr: the destination network not to masquerade for
+ *
+ * Add rules to the IP table context to avoid masquerading from
+ * @netaddr/@prefix to @destaddr on @physdev. @destaddr must be in a format
+ * directly consumable by iptables, it must not depend on user input or
+ * configuration.
+ *
+ * Returns 0 in case of success or an error code otherwise.
+ */
+int
+iptablesAddDontMasquerade(virSocketAddr *netaddr,
+ unsigned int prefix,
+ const char *physdev,
+ const char *destaddr)
+{
+return iptablesForwardDontMasquerade(netaddr, prefix, physdev, destaddr,
Re: [libvirt] [PATCH v4 2/2] bridge driver: don't masquerade local subnet broadcast/multicast packets
On 09/25/2013 06:45 AM, Laszlo Ersek wrote:
Packets sent by guests on virbrN, *or* by dnsmasq on the same, to
- 255.255.255.255/32 (netmask-independent local network broadcast
address), or to
- 224.0.0.0/24 (local subnetwork multicast range)
are never forwarded, hence it is not necessary to masquerade them.
In fact we must not masquerade them: translating their source addresses or
source ports (where applicable) may confuse receivers on virbrN.
One example is the DHCP client in OVMF (= UEFI firmware for virtual
machines):
http://thread.gmane.org/gmane.comp.bios.tianocore.devel/1506/focus=2640
It expects DHCP replies to arrive from remote source port 67. Even though
dnsmasq conforms to that, the destination address (255.255.255.255) and
the source address (eg. 192.168.122.1) in the reply allow the UDP
masquerading rule to match, which rewrites the source port to or above
1024. This prevents the DHCP client in OVMF from accepting the packet.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=709418
Signed-off-by: Laszlo Ersek ler...@redhat.com
---
v2-v4:
- Rename iptables(Add|Remove)ForwardDontMasquerade to
iptables(Add|Remove)DontMasquerade [Laine].
src/network/bridge_driver_linux.c | 70
---
1 file changed, 66 insertions(+), 4 deletions(-)
diff --git a/src/network/bridge_driver_linux.c
b/src/network/bridge_driver_linux.c
index 80430a8..066779a 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -127,6 +127,9 @@ out:
return ret;
}
+static const char networkLocalMulticast[] = 224.0.0.0/24;
+static const char networkLocalBroadcast[] = 255.255.255.255/32;
+
int networkAddMasqueradingFirewallRules(virNetworkObjPtr network,
virNetworkIpDefPtr ipdef)
{
@@ -167,11 +170,20 @@ int
networkAddMasqueradingFirewallRules(virNetworkObjPtr network,
/*
* Enable masquerading.
*
- * We need to end up with 3 rules in the table in this order
+ * We need to end up with 5 rules in the table in this order
*
- * 1. protocol=tcp with sport mapping restriction
- * 2. protocol=udp with sport mapping restriction
- * 3. generic any protocol
+ * 1. do not masquerade packets targeting 224.0.0.0/24
+ * 2. do not masquerade packets targeting 255.255.255.255/32
+ * 3. masquerade protocol=tcp with sport mapping restriction
+ * 4. masquerade protocol=udp with sport mapping restriction
+ * 5. generic, masquerade any protocol
+ *
+ * 224.0.0.0/24 is the local network multicast range. Packets are not
+ * forwarded outside.
+ *
+ * 255.255.255.255/32 is the broadcast address of any local network.
Again,
+ * such packets are never forwarded, but strict DHCP clients don't accept
+ * DHCP replies with changed source ports.
*
* The sport mappings are required, because default IPtables
* MASQUERADE maintain port numbers unchanged where possible.
@@ -238,8 +250,50 @@ int networkAddMasqueradingFirewallRules(virNetworkObjPtr
network,
goto masqerr5;
}
+/* exempt local network broadcast address as destination */
+if (iptablesAddDontMasquerade(ipdef-address,
+ prefix,
+ forwardIf,
+ networkLocalBroadcast) 0) {
+if (forwardIf)
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _(failed to add iptables rule to prevent local
broadcast masquerading on %s),
+ forwardIf);
+else
+virReportError(VIR_ERR_SYSTEM_ERROR, %s,
+ _(failed to add iptables rule to prevent local
broadcast masquerading));
+goto masqerr6;
+}
+
+/* exempt local multicast range as destination */
+if (iptablesAddDontMasquerade(ipdef-address,
+ prefix,
+ forwardIf,
+ networkLocalMulticast) 0) {
+if (forwardIf)
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _(failed to add iptables rule to prevent local
multicast masquerading on %s),
+ forwardIf);
+else
+virReportError(VIR_ERR_SYSTEM_ERROR, %s,
+ _(failed to add iptables rule to prevent local
multicast masquerading));
+goto masqerr7;
+}
+
return 0;
+ masqerr7:
+iptablesRemoveDontMasquerade(ipdef-address,
+ prefix,
+ forwardIf,
+ networkLocalBroadcast);
+ masqerr6:
+iptablesRemoveForwardMasquerade(ipdef-address,
+prefix,
+
Re: [libvirt] [PATCH] virsh-domain: Free dom before return false in cmdDump
On 09/25/2013 07:49 AM, Hongwei Bi wrote:
---
tools/virsh-domain.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 856e888..f6d4edd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4487,8 +4487,10 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, name)))
return false;
-if (vshCommandOptStringReq(ctl, cmd, file, to) 0)
+if (vshCommandOptStringReq(ctl, cmd, file, to) 0) {
+virDomainFree(dom);
return false;
+}
'goto cleanup' would do the same and it's easier to read.
Alternatively, if we moved vshCommandOptStringReq before vshCommandOptDomain,
'return false' would be enough.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh-domain: Free dom before return false in cmdDump
Yes, 'goto cleanup' would be better.
2013/9/25 Hongwei Bi hwbi2...@gmail.com
Yes, 'goto cleanup' would be better.
2013/9/25 Ján Tomko jto...@redhat.com
On 09/25/2013 07:49 AM, Hongwei Bi wrote:
---
tools/virsh-domain.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 856e888..f6d4edd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4487,8 +4487,10 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, name)))
return false;
-if (vshCommandOptStringReq(ctl, cmd, file, to) 0)
+if (vshCommandOptStringReq(ctl, cmd, file, to) 0) {
+virDomainFree(dom);
return false;
+}
'goto cleanup' would do the same and it's easier to read.
Alternatively, if we moved vshCommandOptStringReq before
vshCommandOptDomain,
'return false' would be enough.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh-domain: Free dom before return false in cmdDump
On 09/25/13 13:23, Ján Tomko wrote:
On 09/25/2013 07:49 AM, Hongwei Bi wrote:
---
tools/virsh-domain.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 856e888..f6d4edd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4487,8 +4487,10 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, name)))
return false;
-if (vshCommandOptStringReq(ctl, cmd, file, to) 0)
+if (vshCommandOptStringReq(ctl, cmd, file, to) 0) {
+virDomainFree(dom);
return false;
+}
'goto cleanup' would do the same and it's easier to read.
Alternatively, if we moved vshCommandOptStringReq before vshCommandOptDomain,
'return false' would be enough.
I'd prefer this option.
Jan
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/4] Improve passthrough of early errors from qemu log
On 09/25/13 12:45, Ján Tomko wrote: On 09/24/2013 05:14 PM, Peter Krempa wrote: Version 2 fixed according to review feedback of Jan. Peter Krempa (4): qemu_process: Make qemuProcessReadLog() more versatile and reusable qemu: monitor: Add infrastructure to access VM logs for better err msgs qemu: monitor: Produce better errors on monitor hangup qemu: Wire up better early error reporting src/qemu/qemu_monitor.c | 93 + src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_process.c | 54 +++- src/qemu/qemu_process.h | 2 ++ 4 files changed, 129 insertions(+), 22 deletions(-) ACK Pushed; Thanks for the review. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] regarding libvirt development.
hi, i wish to give my contribution for the libvirt development. Any guide or documentation available for libvirt development. Or Any suggestions you have about the development means, please guide me. Regards cooldharma06 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] qemu: turn if into switch in qemuDomainValidateDevicePCISlotsQ35
This will make it simpler to add checks for other types of
controllers.
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
---
src/qemu/qemu_command.c | 41 +++--
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c8a5c8b..9baed56 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2534,27 +2534,32 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
char *addrStr = NULL;
qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPE_PCIE;
-/* Verify that the first SATA controller is at 00:1F.2 */
-/* the q35 machine type *always* has a SATA controller at this address */
for (i = 0; i def-ncontrollers; i++) {
-if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_SATA
-def-controllers[i]-idx == 0) {
-if (def-controllers[i]-info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-if (def-controllers[i]-info.addr.pci.domain != 0 ||
-def-controllers[i]-info.addr.pci.bus != 0 ||
-def-controllers[i]-info.addr.pci.slot != 0x1F ||
-def-controllers[i]-info.addr.pci.function != 2) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(Primary SATA controller must have PCI
address 0:0:1f.2));
-goto cleanup;
+switch (def-controllers[i]-type) {
+case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
+/* Verify that the first SATA controller is at 00:1F.2 the
+ * q35 machine type *always* has a SATA controller at this
+ * address.
+ */
+if (def-controllers[i]-idx == 0) {
+if (def-controllers[i]-info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+if (def-controllers[i]-info.addr.pci.domain != 0 ||
+def-controllers[i]-info.addr.pci.bus != 0 ||
+def-controllers[i]-info.addr.pci.slot != 0x1F ||
+def-controllers[i]-info.addr.pci.function != 2) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Primary SATA controller must have
PCI address 0:0:1f.2));
+goto cleanup;
+}
+} else {
+def-controllers[i]-info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+def-controllers[i]-info.addr.pci.domain = 0;
+def-controllers[i]-info.addr.pci.bus = 0;
+def-controllers[i]-info.addr.pci.slot = 0x1F;
+def-controllers[i]-info.addr.pci.function = 2;
}
-} else {
-def-controllers[i]-info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-def-controllers[i]-info.addr.pci.domain = 0;
-def-controllers[i]-info.addr.pci.bus = 0;
-def-controllers[i]-info.addr.pci.slot = 0x1F;
-def-controllers[i]-info.addr.pci.function = 2;
}
+break;
}
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] qemu: eliminate redundant if clauses in qemuCollectPCIAddress
Replace them with switch cases. This will make it more efficient when
we add exceptions more controller types, and other device types.
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
---
src/qemu/qemu_command.c | 57 +++--
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0376611..2683a9c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1731,37 +1731,44 @@ qemuCollectPCIAddress(virDomainDefPtr def
ATTRIBUTE_UNUSED,
/* Change flags according to differing requirements of different
* devices.
*/
-if (device-type == VIR_DOMAIN_DEVICE_CONTROLLER
-device-data.controller-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
-switch (device-data.controller-model) {
-case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
-/* pci-bridge needs a PCI slot, but it isn't
- * hot-pluggable, so it doesn't need a hot-pluggable slot.
- */
-flags = QEMU_PCI_CONNECT_TYPE_PCI;
+switch (device-type) {
+case VIR_DOMAIN_DEVICE_CONTROLLER:
+switch (device-data.controller-type) {
+case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+switch (device-data.controller-model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+/* pci-bridge needs a PCI slot, but it isn't
+ * hot-pluggable, so it doesn't need a hot-pluggable slot.
+ */
+flags = QEMU_PCI_CONNECT_TYPE_PCI;
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+/* pci-bridge needs a PCIe slot, but it isn't
+ * hot-pluggable, so it doesn't need a hot-pluggable slot.
+ */
+flags = QEMU_PCI_CONNECT_TYPE_PCIE;
+break;
+default:
+break;
+}
break;
-case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
-/* pci-bridge needs a PCIe slot, but it isn't
- * hot-pluggable, so it doesn't need a hot-pluggable slot.
+
+case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
+/* SATA controllers aren't hot-plugged, and can be put in
+ * either a PCI or PCIe slot
*/
-flags = QEMU_PCI_CONNECT_TYPE_PCIE;
-break;
-default:
+flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
break;
}
-}
-/* SATA controllers aren't hot-plugged, and can be put in either a
- * PCI or PCIe slot
- */
-if (device-type == VIR_DOMAIN_DEVICE_CONTROLLER
-device-data.controller-type == VIR_DOMAIN_CONTROLLER_TYPE_SATA)
-flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
+break;
-/* video cards aren't hot-plugged, and can be put in either a PCI
- * or PCIe slot
- */
-if (device-type == VIR_DOMAIN_DEVICE_VIDEO)
+case VIR_DOMAIN_DEVICE_VIDEO:
+/* video cards aren't hot-plugged, and can be put in either a
+ * PCI or PCIe slot
+ */
flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
+break;
+}
/* Ignore implicit controllers on slot 0:0:1.0:
* implicit IDE controller on 0:0:1.1 (no qemu command line)
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] Several Q35 related tweaks
These patches are all in response to the issues that Gerd Hoffman raised in https://bugzilla.redhat.com/show_bug.cgi?id=1003983 Three of the six are simple refactoring of existing code, done separately so that the patches that change functionality would be simpler to review. None of them is very long. Laine Stump (6): qemu: eliminate redundant if clauses in qemuCollectPCIAddress qemu: allow some PCI devices to be attached to PCIe slots qemu: replace multiple strcmps with a switch on an enum qemu: support ich9-intel-hda audio device qemu: turn if into switch in qemuDomainValidateDevicePCISlotsQ35 qemu: prefer to put a Q35 machine's dmi-to-pci-bridge at 00:1E.0 docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 6 +- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 197 +++-- src/qemu/qemu_command.h| 4 + tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 2 +- .../qemuxml2argv-pcihole64-q35.args| 2 +- tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +- .../qemuxml2argv-sound-device.args | 7 +- .../qemuxml2argvdata/qemuxml2argv-sound-device.xml | 5 + tests/qemuxml2argvtest.c | 3 +- 13 files changed, 175 insertions(+), 58 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] qemu: allow some PCI devices to be attached to PCIe slots
Part of the resolution to:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
Although most devices available in qemu area defined as PCI devices,
and strictly speaking should only be attached via a PCI slot, in
practice qemu allows them to be attached to a PCIe slot and sometimes
this makes sense.
For example, The UHCI and EHCI USB controllers are usually attached
directly to the PCIe root complex (i.e. PCIe slots) on real
hardware, so that should be possible for a Q35-based qemu virtual
machine as well.
We still want to prefer a standard PCi slot when auto-assigning
addresses, though, and in general to disallow attaching PCI devices
via PCIe slots.
This patch makes that possible by adding a new
QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG flag. Three things are done
with this flag:
1) It is set for the pcie-root controller
2) qemuCollectPCIAddress() now has a set of nested switches that set
this EITHER flag for devices that we want to allow connecting to
pcie-root when specifically requested in the config.
3) qemuDomainPCIAddressFlagsCompatible() adds this new flag to the
flagsMatchMask if the address being checked came from config rather
than being newly auto-allocated by libvirt (this knowledge is
conveniently already available in the fromConfig arg).
Now any device having the EITHER flag set can be connected to
pcie-root if explicitly requested, but auto-allocated addresses for
those devices will still be standard PCI slots instead.
This patch only loosens the restrictions on devices that have been
specifically requested, but the setup is such that it should be fairly
easy to add new devices.
---
src/qemu/qemu_command.c | 50 ++---
src/qemu/qemu_command.h | 4
2 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2683a9c..280d8d2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1496,12 +1496,16 @@
qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
{
virErrorNumber errType = (fromConfig
? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
+qemuDomainPCIConnectFlags flagsMatchMask = QEMU_PCI_CONNECT_TYPES_MASK;
+
+if (fromConfig)
+ flagsMatchMask |= QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG;
/* If this bus doesn't allow the type of connection (PCI
* vs. PCIe) required by the device, or if the device requires
* hot-plug and this bus doesn't have it, return false.
*/
-if (!(devFlags busFlags QEMU_PCI_CONNECT_TYPES_MASK)) {
+if (!(devFlags busFlags flagsMatchMask)) {
if (reportError) {
if (devFlags QEMU_PCI_CONNECT_TYPE_PCI) {
virReportError(errType,
@@ -1620,8 +1624,12 @@
qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus,
bus-maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
-/* slots 1 - 31, PCIe devices only, no hotplug */
-bus-flags = QEMU_PCI_CONNECT_TYPE_PCIE;
+/* slots 1 - 31, no hotplug, PCIe only unless the address was
+ * specified in user config *and* the particular device being
+ * attached also allows it
+ */
+bus-flags = (QEMU_PCI_CONNECT_TYPE_PCIE |
+ QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
bus-minSlot = 1;
bus-maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
break;
@@ -1759,6 +1767,42 @@ qemuCollectPCIAddress(virDomainDefPtr def
ATTRIBUTE_UNUSED,
*/
flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
break;
+
+case VIR_DOMAIN_CONTROLLER_TYPE_USB:
+ /* allow UHCI and EHCI controllers to be manually placed on
+* the PCIe bus (but don't put them there automatically)
+*/
+ switch (device-data.controller-model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI:
+ flags = (QEMU_PCI_CONNECT_TYPE_PCI |
+ QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
+ /* should this be PCIE-only? Or do we need to allow PCI
+ * for backward compatibility?
+ */
+ flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI:
+ /* Allow these for PCI only */
+ break;
+
[libvirt] [PATCH 3/6] qemu: replace multiple strcmps with a switch on an enum
I'm not sure why this code was written to compare the strings that it
had just retrieved from an enum-string conversion, rather than just
look at the original enum values, but this yields the same results,
and is much more efficient (especially as you add more devices).
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
---
src/qemu/qemu_command.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 280d8d2..3156cff 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5269,13 +5269,18 @@ qemuBuildSoundDevStr(virDomainDefPtr def,
goto error;
}
-/* Hack for weirdly unusual devices name in QEMU */
-if (STREQ(model, es1370))
+/* Hack for devices with different names in QEMU and libvirt */
+switch (sound-model) {
+case VIR_DOMAIN_SOUND_MODEL_ES1370:
model = ES1370;
-else if (STREQ(model, ac97))
+break;
+case VIR_DOMAIN_SOUND_MODEL_AC97:
model = AC97;
-else if (STREQ(model, ich6))
+break;
+case VIR_DOMAIN_SOUND_MODEL_ICH6:
model = intel-hda;
+break;
+}
virBufferAsprintf(buf, %s,id=%s, model, sound-info.alias);
if (qemuBuildDeviceAddressStr(buf, def, sound-info, qemuCaps) 0)
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] qemu: prefer to put a Q35 machine's dmi-to-pci-bridge at 00:1E.0
This resolves one of the issues listed in:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
00:1E.0 is the location of this controller on at least some actual Q35
hardware, so we try to replicate the placement. The bridge should work
just as well in any other location though, so if 00:1E.0 isn't
available, just allow it to be auto-assigned anywhere appropriate.
---
src/qemu/qemu_command.c| 22 ++
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 2 +-
.../qemuxml2argv-pcihole64-q35.args| 2 +-
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +-
4 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9baed56..38dd451 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2560,6 +2560,28 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
}
}
break;
+
+case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+if (def-controllers[i]-model ==
VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE
+def-controllers[i]-info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+/* Try to assign this bridge to 00:1E.0 (because that
+* is its standard location on real hardware) unless
+* it's already taken, but don't insist on it.
+*/
+memset(tmp_addr, 0, sizeof(tmp_addr));
+tmp_addr.slot = 0x1E;
+if (!qemuDomainPCIAddressSlotInUse(addrs, tmp_addr)) {
+if (qemuDomainPCIAddressReserveAddr(addrs, tmp_addr,
+flags, true, false)
0)
+goto cleanup;
+def-controllers[i]-info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+def-controllers[i]-info.addr.pci.domain = 0;
+def-controllers[i]-info.addr.pci.bus = 0;
+def-controllers[i]-info.addr.pci.slot = 0x1E;
+def-controllers[i]-info.addr.pci.function = 0;
+}
+}
+break;
}
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
index 010e089..48c21cd 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
@@ -2,5 +2,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
/usr/libexec/qemu-kvm \
-S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
--device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x2 \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args
b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args
index 175c0dc..6855cd2 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args
@@ -2,7 +2,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
-monitor unix:/tmp/test-monitor,server,nowait -no-acpi \
-boot c -global q35-pcihost.pci-hole64-size=1048576K \
--device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x2 \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \
-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
index 6bd5f4c..8cc5874 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
@@ -1,7 +1,7 @@
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
--device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x2 \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \
-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] qemu: support ich9-intel-hda audio device
This resolves one of the issues in:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
This device is identical to qemu's intel-hda device (known as ich6
in libvirt), but has a different PCI device ID (which matches the ID
of the hda audio built into the ich9 chipset, of course). It's not
supported int earlier versions of qemu, so it requires a capability
bit.
---
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c| 6 --
src/conf/domain_conf.h| 1 +
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 16 ++--
tests/qemuxml2argvdata/qemuxml2argv-sound-device.args | 7 ++-
tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml | 5 +
tests/qemuxml2argvtest.c | 3 ++-
9 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index fb6a26c..5c5301d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2947,6 +2947,7 @@
valuepcspk/value
valueac97/value
valueich6/value
+ valueich9/value
/choice
/attribute
interleave
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 240f318..4110127 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -462,7 +462,8 @@ VIR_ENUM_IMPL(virDomainSoundModel,
VIR_DOMAIN_SOUND_MODEL_LAST,
es1370,
pcspk,
ac97,
- ich6)
+ ich6,
+ ich9)
VIR_ENUM_IMPL(virDomainMemDump, VIR_DOMAIN_MEM_DUMP_LAST,
default,
@@ -8451,7 +8452,8 @@ virDomainSoundDefParseXML(const xmlNodePtr node,
goto error;
}
-if (def-model == VIR_DOMAIN_SOUND_MODEL_ICH6) {
+if (def-model == VIR_DOMAIN_SOUND_MODEL_ICH6 ||
+def-model == VIR_DOMAIN_SOUND_MODEL_ICH9) {
int ncodecs;
xmlNodePtr *codecNodes = NULL;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5c33e08..f20a916 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1253,6 +1253,7 @@ enum virDomainSoundModel {
VIR_DOMAIN_SOUND_MODEL_PCSPK,
VIR_DOMAIN_SOUND_MODEL_AC97,
VIR_DOMAIN_SOUND_MODEL_ICH6,
+VIR_DOMAIN_SOUND_MODEL_ICH9,
VIR_DOMAIN_SOUND_MODEL_LAST
};
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d830e2a..dc8f0be 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -241,6 +241,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
usb-storage, /* 155 */
usb-storage.removable,
virtio-mmio,
+ ich9-intel-hda,
);
struct _virQEMUCaps {
@@ -1391,6 +1392,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ i82801b11-bridge, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE },
{ usb-storage, QEMU_CAPS_DEVICE_USB_STORAGE },
{ virtio-mmio, QEMU_CAPS_DEVICE_VIRTIO_MMIO },
+{ ich9-intel-hda, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f3c8fa8..f8dc728 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -196,6 +196,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_DEVICE_USB_STORAGE = 155, /* -device usb-storage */
QEMU_CAPS_USB_STORAGE_REMOVABLE = 156, /* usb-storage.removable */
QEMU_CAPS_DEVICE_VIRTIO_MMIO = 157, /* -device virtio-mmio */
+QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3156cff..c8a5c8b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1800,6 +1800,7 @@ qemuCollectPCIAddress(virDomainDefPtr def
ATTRIBUTE_UNUSED,
case VIR_DOMAIN_DEVICE_SOUND:
switch (device-data.sound-model) {
case VIR_DOMAIN_SOUND_MODEL_ICH6:
+case VIR_DOMAIN_SOUND_MODEL_ICH9:
flags = (QEMU_PCI_CONNECT_TYPE_PCI |
QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
break;
@@ -5280,6 +5281,15 @@ qemuBuildSoundDevStr(virDomainDefPtr def,
case VIR_DOMAIN_SOUND_MODEL_ICH6:
model = intel-hda;
break;
+case VIR_DOMAIN_SOUND_MODEL_ICH9:
+model = ich9-intel-hda;
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(The ich9-intel-hda audio controller
+ is not supported in this QEMU binary));
+goto error;
+}
+break;
}
Re: [libvirt] [PATCH 5/6] qemu: turn if into switch in qemuDomainValidateDevicePCISlotsQ35
On 09/25/2013 02:30 PM, Laine Stump wrote: This will make it simpler to add checks for other types of controllers. This is a prerequisite for patches to resolve: https://bugzilla.redhat.com/show_bug.cgi?id=1003983 --- src/qemu/qemu_command.c | 41 +++-- 1 file changed, 23 insertions(+), 18 deletions(-) ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] qemu: eliminate redundant if clauses in qemuCollectPCIAddress
On 09/25/2013 02:30 PM, Laine Stump wrote: Replace them with switch cases. This will make it more efficient when we add exceptions more controller types, and other device types. s/more/for more/ This is a prerequisite for patches to resolve: https://bugzilla.redhat.com/show_bug.cgi?id=1003983 --- src/qemu/qemu_command.c | 57 +++-- 1 file changed, 32 insertions(+), 25 deletions(-) ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] qemu: replace multiple strcmps with a switch on an enum
On 09/25/2013 02:30 PM, Laine Stump wrote: I'm not sure why this code was written to compare the strings that it had just retrieved from an enum-string conversion, rather than just look at the original enum values, but this yields the same results, and is much more efficient (especially as you add more devices). This is a prerequisite for patches to resolve: https://bugzilla.redhat.com/show_bug.cgi?id=1003983 --- src/qemu/qemu_command.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemu: allow some PCI devices to be attached to PCIe slots
On 09/25/2013 02:30 PM, Laine Stump wrote: Part of the resolution to: https://bugzilla.redhat.com/show_bug.cgi?id=1003983 Although most devices available in qemu area defined as PCI devices, and strictly speaking should only be attached via a PCI slot, in practice qemu allows them to be attached to a PCIe slot and sometimes this makes sense. For example, The UHCI and EHCI USB controllers are usually attached directly to the PCIe root complex (i.e. PCIe slots) on real hardware, so that should be possible for a Q35-based qemu virtual machine as well. We still want to prefer a standard PCi slot when auto-assigning *PCI addresses, though, and in general to disallow attaching PCI devices via PCIe slots. This patch makes that possible by adding a new QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG flag. Three things are done with this flag: 1) It is set for the pcie-root controller 2) qemuCollectPCIAddress() now has a set of nested switches that set this EITHER flag for devices that we want to allow connecting to pcie-root when specifically requested in the config. 3) qemuDomainPCIAddressFlagsCompatible() adds this new flag to the flagsMatchMask if the address being checked came from config rather than being newly auto-allocated by libvirt (this knowledge is conveniently already available in the fromConfig arg). Now any device having the EITHER flag set can be connected to pcie-root if explicitly requested, but auto-allocated addresses for those devices will still be standard PCI slots instead. This patch only loosens the restrictions on devices that have been specifically requested, but the setup is such that it should be fairly easy to add new devices. --- src/qemu/qemu_command.c | 50 ++--- src/qemu/qemu_command.h | 4 2 files changed, 51 insertions(+), 3 deletions(-) The code looks OK, and it seems to work with my limited testing. ACK, but I don't know the answer to that USB_NEC_XHCI question. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] qemu: prefer to put a Q35 machine's dmi-to-pci-bridge at 00:1E.0
On 09/25/2013 02:30 PM, Laine Stump wrote: This resolves one of the issues listed in: https://bugzilla.redhat.com/show_bug.cgi?id=1003983 00:1E.0 is the location of this controller on at least some actual Q35 hardware, so we try to replicate the placement. The bridge should work just as well in any other location though, so if 00:1E.0 isn't available, just allow it to be auto-assigned anywhere appropriate. --- src/qemu/qemu_command.c| 22 ++ tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 2 +- .../qemuxml2argv-pcihole64-q35.args| 2 +- tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +- 4 files changed, 25 insertions(+), 3 deletions(-) ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] qemu: support ich9-intel-hda audio device
On 09/25/2013 02:30 PM, Laine Stump wrote: This resolves one of the issues in: https://bugzilla.redhat.com/show_bug.cgi?id=1003983 This device is identical to qemu's intel-hda device (known as ich6 in libvirt), but has a different PCI device ID (which matches the ID of the hda audio built into the ich9 chipset, of course). It's not supported int earlier versions of qemu, so it requires a capability *in bit. --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 6 -- src/conf/domain_conf.h| 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 16 ++-- tests/qemuxml2argvdata/qemuxml2argv-sound-device.args | 7 ++- tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml | 5 + tests/qemuxml2argvtest.c | 3 ++- 9 files changed, 36 insertions(+), 6 deletions(-) ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] regarding libvirt development.
On 09/24/2013 11:39 PM, cooldharma06 wrote: hi, i wish to give my contribution for the libvirt development. Any guide or documentation available for libvirt development. Or Any suggestions you have about the development means, please guide me. Welcome! http://libvirt.org/hacking.html is a good start, with hints for setting up your development environment and how to submit a patch. Beyond that, feel free to review other patches on this list (as a great way to learn the code base) and submit patches for whatever strikes your fancy. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] Several Q35 related tweaks
On 09/25/2013 08:30 AM, Laine Stump wrote: These patches are all in response to the issues that Gerd Hoffman raised in https://bugzilla.redhat.com/show_bug.cgi?id=1003983 Three of the six are simple refactoring of existing code, done separately so that the patches that change functionality would be simpler to review. None of them is very long. Laine Stump (6): qemu: eliminate redundant if clauses in qemuCollectPCIAddress qemu: allow some PCI devices to be attached to PCIe slots qemu: replace multiple strcmps with a switch on an enum qemu: support ich9-intel-hda audio device qemu: turn if into switch in qemuDomainValidateDevicePCISlotsQ35 qemu: prefer to put a Q35 machine's dmi-to-pci-bridge at 00:1E.0 Thanks for the reviews. I've pushed the series. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/23] Fix crash on OOM in xenParseSxpr
From: Daniel P. Berrange berra...@redhat.com
The xenParseSxpr method sets def-nconsoles to 1 before allocating
the def-consoles array. If the allocation fails due to OOM the
cleanup code will thus crash accessing out of bounds.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xenxs/xen_sxpr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index 6209c68..462eac6 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1439,9 +1439,9 @@ xenParseSxpr(const struct sexpr *root,
def-parallels[def-nparallels++] = chr;
}
} else if (def-id != 0) {
-def-nconsoles = 1;
if (VIR_ALLOC_N(def-consoles, 1) 0)
goto error;
+def-nconsoles = 1;
/* Fake a paravirt console, since that's not in the sexpr */
if (!(def-consoles[0] = xenParseSxprChar(pty, tty)))
goto error;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/23] Don't clobber 'ret' in LXC XML test case
From: Daniel P. Berrange berra...@redhat.com
The testCompareXMLToXMLHelper method clobbered the 'ret' variable
in several places leading to a failure to report OOM errors from
the test suite.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tests/lxcxml2xmltest.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c
index ca05d29..aeb3940 100644
--- a/tests/lxcxml2xmltest.c
+++ b/tests/lxcxml2xmltest.c
@@ -79,18 +79,23 @@ testCompareXMLToXMLHelper(const void *data)
goto cleanup;
if (info-different) {
-ret = testCompareXMLToXMLFiles(xml_in, xml_out, false);
+if (testCompareXMLToXMLFiles(xml_in, xml_out, false) 0)
+goto cleanup;
} else {
-ret = testCompareXMLToXMLFiles(xml_in, xml_in, false);
+if (testCompareXMLToXMLFiles(xml_in, xml_in, false) 0)
+goto cleanup;
}
if (!info-inactive_only) {
if (info-different) {
-ret = testCompareXMLToXMLFiles(xml_in, xml_out, true);
+if (testCompareXMLToXMLFiles(xml_in, xml_out, true) 0)
+goto cleanup;
} else {
-ret = testCompareXMLToXMLFiles(xml_in, xml_in, true);
+if (testCompareXMLToXMLFiles(xml_in, xml_in, true) 0)
+goto cleanup;
}
}
+ret = 0;
cleanup:
VIR_FREE(xml_in);
VIR_FREE(xml_out);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/23] Fix handling of OOM when getting Xen dom ID
From: Daniel P. Berrange berra...@redhat.com
The methods for obtaining the Xen dom ID cannot distinguish
between returning -1 due to an error and returning -1 due to
the domain being shutoff. Change them to return the dom ID
via an output parameter.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c| 3 ++-
src/xen/xend_internal.c | 10 ++
src/xenxs/xen_sxpr.c| 17 ++---
src/xenxs/xen_sxpr.h| 4 ++--
tests/sexpr2xmltest.c | 3 ++-
5 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 6cb4f4f..40b98ee 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1604,7 +1604,8 @@ xenUnifiedConnectDomainXMLFromNative(virConnectPtr conn,
def = xenParseXM(conf, priv-xendConfigVersion, priv-caps);
} else if (STREQ(format, XEN_CONFIG_FORMAT_SEXPR)) {
-id = xenGetDomIdFromSxprString(config, priv-xendConfigVersion);
+if (xenGetDomIdFromSxprString(config, priv-xendConfigVersion, id)
0)
+goto cleanup;
xenUnifiedLock(priv);
tty = xenStoreDomainGetConsolePath(conn, id);
vncport = xenStoreDomainGetVNCPort(conn, id);
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index f698c8d..dc57350 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1561,7 +1561,7 @@ xenDaemonDomainFetch(virConnectPtr conn, int domid, const
char *name,
{
struct sexpr *root;
xenUnifiedPrivatePtr priv = conn-privateData;
-virDomainDefPtr def;
+virDomainDefPtr def = NULL;
int id;
char * tty;
int vncport;
@@ -1573,7 +1573,8 @@ xenDaemonDomainFetch(virConnectPtr conn, int domid, const
char *name,
if (root == NULL)
return NULL;
-id = xenGetDomIdFromSxpr(root, priv-xendConfigVersion);
+if (xenGetDomIdFromSxpr(root, priv-xendConfigVersion, id) 0)
+goto cleanup;
xenUnifiedLock(priv);
if (sexpr_lookup(root, domain/image/hvm))
tty = xenStoreDomainGetSerialConsolePath(conn, id);
@@ -3224,7 +3225,7 @@ xenDaemonDomainBlockPeek(virConnectPtr conn,
xenUnifiedPrivatePtr priv = conn-privateData;
struct sexpr *root = NULL;
int fd = -1, ret = -1;
-virDomainDefPtr def;
+virDomainDefPtr def = NULL;
int id;
char * tty;
int vncport;
@@ -3249,7 +3250,8 @@ xenDaemonDomainBlockPeek(virConnectPtr conn,
return -1;
}
-id = xenGetDomIdFromSxpr(root, priv-xendConfigVersion);
+if (xenGetDomIdFromSxpr(root, priv-xendConfigVersion, id) 0)
+goto cleanup;
xenUnifiedLock(priv);
tty = xenStoreDomainGetConsolePath(conn, id);
vncport = xenStoreDomainGetVNCPort(conn, id);
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index 462eac6..bb8a335 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -40,30 +40,33 @@
#include virstring.h
/* Get a domain id from a S-expression string */
-int xenGetDomIdFromSxprString(const char *sexpr, int xendConfigVersion)
+int xenGetDomIdFromSxprString(const char *sexpr, int xendConfigVersion, int
*id)
{
struct sexpr *root = string2sexpr(sexpr);
+int ret;
+
+*id = -1;
if (!root)
return -1;
-int id = xenGetDomIdFromSxpr(root, xendConfigVersion);
+ret = xenGetDomIdFromSxpr(root, xendConfigVersion, id);
sexpr_free(root);
-return id;
+return ret;
}
/* Get a domain id from a S-expression */
-int xenGetDomIdFromSxpr(const struct sexpr *root, int xendConfigVersion)
+int xenGetDomIdFromSxpr(const struct sexpr *root, int xendConfigVersion, int
*id)
{
-int id = -1;
const char * tmp = sexpr_node(root, domain/domid);
if (tmp == NULL xendConfigVersion XEND_CONFIG_VERSION_3_0_4) { /*
domid was mandatory */
virReportError(VIR_ERR_INTERNAL_ERROR,
%s, _(domain information incomplete, missing id));
+return -1;
} else {
- id = tmp ? sexpr_int(root, domain/domid) : -1;
+*id = tmp ? sexpr_int(root, domain/domid) : -1;
+return 0;
}
-return id;
}
/*
diff --git a/src/xenxs/xen_sxpr.h b/src/xenxs/xen_sxpr.h
index d7ce46a..f354a50 100644
--- a/src/xenxs/xen_sxpr.h
+++ b/src/xenxs/xen_sxpr.h
@@ -40,8 +40,8 @@ typedef enum {
} xenConfigVersionEnum;
/* helper functions to get the dom id from a sexpr */
-int xenGetDomIdFromSxprString(const char *sexpr, int xendConfigVersion);
-int xenGetDomIdFromSxpr(const struct sexpr *root, int xendConfigVersion);
+int xenGetDomIdFromSxprString(const char *sexpr, int xendConfigVersion, int
*id);
+int xenGetDomIdFromSxpr(const struct sexpr *root, int xendConfigVersion, int
*id);
virDomainDefPtr xenParseSxprString(const char *sexpr, int xendConfigVersion,
char *tty, int vncport);
diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c
index eafefda..b939319 100644
[libvirt] [PATCH 14/23] Fix leak in virLockSpaceResourceFree
From: Daniel P. Berrange berra...@redhat.com Normally a lockspace resource is not freed while there are active owners. During initial resource creation though, an OOM error will trigger this scenario. virLockSpaceResourceFree was not freeing the 'owners' field in this case. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/util/virlockspace.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index afb1abb..cab7775 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -102,6 +102,7 @@ static void virLockSpaceResourceFree(virLockSpaceResourcePtr res) } } +VIR_FREE(res-owners); VIR_FORCE_CLOSE(res-fd); VIR_FREE(res-path); VIR_FREE(res-name); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 18/23] Avoid uninitialized data in qemuMonitorTestNew
From: Daniel P. Berrange berra...@redhat.com The virDomainChrSourceDef variable should be memset to 0, so that the cleanup block does not free uninitialized data on OOM. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/qemumonitortestutils.c | 4 1 file changed, 4 insertions(+) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 9568476..bca3385 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -878,6 +878,8 @@ qemuMonitorTestNew(bool json, qemuMonitorTestPtr test = NULL; virDomainChrSourceDef src; +memset(src, 0, sizeof(src)); + if (!(test = qemuMonitorCommonTestNew(xmlopt, vm, src))) goto error; @@ -915,6 +917,8 @@ qemuMonitorTestNewAgent(virDomainXMLOptionPtr xmlopt) qemuMonitorTestPtr test = NULL; virDomainChrSourceDef src; +memset(src, 0, sizeof(src)); + if (!(test = qemuMonitorCommonTestNew(xmlopt, NULL, src))) goto error; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 17/23] Avoid double free in qemuMonitorCommonTestInit on OOM
From: Daniel P. Berrange berra...@redhat.com The qemuMonitorCommonTestInit method did not allocate the test object, so it should not free it upon failure. Doing so causes a double free with the caller. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/qemumonitortestutils.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 763102c..9568476 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -849,7 +849,6 @@ qemuMonitorCommonTestInit(qemuMonitorTestPtr test) return 0; error: -qemuMonitorTestFree(test); return -1; } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/23] Fix crash on OOM in virDomainSnapshotDefParse
From: Daniel P. Berrange berra...@redhat.com
The virDomainSnapshotDefParse method assigned to def-ndisks
before allocating def-disks. Thus if an OOM occurred, the
cleanup code would access out of bounds.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/conf/snapshot_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 45d6af4..207a8fe 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -303,9 +303,9 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
if ((n = virXPathNodeSet(./disks/*, ctxt, nodes)) 0)
goto cleanup;
if (flags VIR_DOMAIN_SNAPSHOT_PARSE_DISKS) {
-def-ndisks = n;
-if (def-ndisks VIR_ALLOC_N(def-disks, def-ndisks) 0)
+if (n VIR_ALLOC_N(def-disks, n) 0)
goto cleanup;
+def-ndisks = n;
for (i = 0; i def-ndisks; i++) {
if (virDomainSnapshotDiskDefParseXML(nodes[i], def-disks[i]) 0)
goto cleanup;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/23] Fix yet more OOM errors
From: Daniel P. Berrange berra...@redhat.com A nice mix of leaks and crashes and ignoring errors. Daniel P. Berrange (23): Fix crash on OOM in xenParseSxpr Fix handling of OOM when getting Xen dom ID Don't clobber return value in virInterfaceDefParseProtoIPv6 Fix crash on OOM in virDomainSnapshotDefParse Don't clobber 'ret' in LXC XML test case Fix double free of hostdev on OOM in xenParseSxprPCI Fix crash on OOM parsing storage pool XML Add missing check for OOM with virVMXEscapeHexPipe Fix leak of comment string if virConfAddEntry fails on OOM Don't print all test suite errors to stderr in vmx2xmltest Fix leak of iterators in virDBusMessageIterEncode Fix double-free in virJSONParserHandleStartMap on OOM Fix leak of parser state in virJSONValueFromString Fix leak in virLockSpaceResourceFree Don't ignore errors parsing nwfilter rules Fix leak on OOM in qemuMonitorCommonTestNew Avoid double free in qemuMonitorCommonTestInit on OOM Avoid uninitialized data in qemuMonitorTestNew Avoid crash on OOM in virbuftest Avoid crash on OOM in virlockspacetest Avoid crash on OOM in virportallocatortest Avoid crash on OOM in virnetmessagetest Avoid use of uninitialized data in virnetmessagetest src/conf/interface_conf.c| 16 ++ src/conf/nwfilter_conf.c | 16 +++--- src/conf/snapshot_conf.c | 4 ++-- src/conf/storage_conf.c | 6 +++--- src/util/virconf.c | 5 - src/util/virdbus.c | 24 ++--- src/util/virjson.c | 2 +- src/util/virlockspace.c | 1 + src/vmx/vmx.c| 3 ++- src/xen/xen_driver.c | 3 ++- src/xen/xend_internal.c | 10 + src/xenxs/xen_sxpr.c | 23 ++-- src/xenxs/xen_sxpr.h | 4 ++-- tests/lxcxml2xmltest.c | 13 +++ tests/nwfilterxml2xmltest.c | 14 ++-- tests/qemumonitortestutils.c | 7 +- tests/sexpr2xmltest.c| 3 ++- tests/virbuftest.c | 14 tests/virlockspacetest.c | 21 -- tests/virnetmessagetest.c| 9 +++- tests/virportallocatortest.c | 6 ++ tests/vmx2xmltest.c | 51 +--- 22 files changed, 154 insertions(+), 101 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/23] Don't print all test suite errors to stderr in vmx2xmltest
From: Daniel P. Berrange berra...@redhat.com
The vmx2xmltest test would print all errors to stderr, which
is not helpful when running OOM tests, and differs from the
behaviour of other tests.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tests/vmx2xmltest.c | 51 +++
1 file changed, 19 insertions(+), 32 deletions(-)
diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 00bf21d..86272ce 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -73,51 +73,38 @@ testCapsInit(void)
static int
testCompareFiles(const char *vmx, const char *xml)
{
-int result = -1;
+int ret = -1;
char *vmxData = NULL;
char *xmlData = NULL;
char *formatted = NULL;
virDomainDefPtr def = NULL;
-virErrorPtr err = NULL;
-if (virtTestLoadFile(vmx, vmxData) 0) {
-goto failure;
-}
-
-if (virtTestLoadFile(xml, xmlData) 0) {
-goto failure;
-}
-
-def = virVMXParseConfig(ctx, xmlopt, vmxData);
+if (virtTestLoadFile(vmx, vmxData) 0)
+goto cleanup;
-if (def == NULL) {
-err = virGetLastError();
-fprintf(stderr, ERROR: %s\n, err != NULL ? err-message :
unknown);
-goto failure;
-}
+if (virtTestLoadFile(xml, xmlData) 0)
+goto cleanup;
-formatted = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE);
+if (!(def = virVMXParseConfig(ctx, xmlopt, vmxData)))
+goto cleanup;
-if (formatted == NULL) {
-err = virGetLastError();
-fprintf(stderr, ERROR: %s\n, err != NULL ? err-message :
unknown);
-goto failure;
-}
+if (!(formatted = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE)))
+goto cleanup;
if (STRNEQ(xmlData, formatted)) {
virtTestDifference(stderr, xmlData, formatted);
-goto failure;
+goto cleanup;
}
-result = 0;
+ret = 0;
- failure:
+ cleanup:
VIR_FREE(vmxData);
VIR_FREE(xmlData);
VIR_FREE(formatted);
virDomainDefFree(def);
-return result;
+return ret;
}
struct testInfo {
@@ -128,7 +115,7 @@ struct testInfo {
static int
testCompareHelper(const void *data)
{
-int result = -1;
+int ret = -1;
const struct testInfo *info = data;
char *vmx = NULL;
char *xml = NULL;
@@ -140,13 +127,13 @@ testCompareHelper(const void *data)
goto cleanup;
}
-result = testCompareFiles(vmx, xml);
+ret = testCompareFiles(vmx, xml);
cleanup:
VIR_FREE(vmx);
VIR_FREE(xml);
-return result;
+return ret;
}
static char *
@@ -195,7 +182,7 @@ testParseVMXFileName(const char *fileName, void *opaque
ATTRIBUTE_UNUSED)
static int
mymain(void)
{
-int result = 0;
+int ret = 0;
# define DO_TEST(_in, _out) \
do { \
@@ -203,7 +190,7 @@ mymain(void)
virResetLastError(); \
if (virtTestRun(VMware VMX-2-XML _in - _out, 1, \
testCompareHelper, info) 0) { \
-result = -1; \
+ret = -1; \
} \
} while (0)
@@ -299,7 +286,7 @@ mymain(void)
virObjectUnref(caps);
virObjectUnref(xmlopt);
-return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
VIRT_TEST_MAIN(mymain)
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/23] Fix double-free in virJSONParserHandleStartMap on OOM
From: Daniel P. Berrange berra...@redhat.com
If OOM occurs in virJSONParserHandleStartMap it will free
a variable that is owned by another object. This leads to
a later double-free.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virjson.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index e93def7..8918bc7 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -862,7 +862,6 @@ static int virJSONParserHandleStartMap(void *ctx)
if (VIR_REALLOC_N(parser-state,
parser-nstate + 1) 0) {
-virJSONValueFree(value);
return 0;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/23] Fix double free of hostdev on OOM in xenParseSxprPCI
From: Daniel P. Berrange berra...@redhat.com
If xenParseSxprPCI failed to expand the def-hostdevs array
due to OOM, it would free the hostdev instance twice.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xenxs/xen_sxpr.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index bb8a335..3cbe958 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1055,10 +1055,8 @@ xenParseSxprPCI(virDomainDefPtr def,
dev-source.subsys.u.pci.addr.slot = slotID;
dev-source.subsys.u.pci.addr.function = funcID;
-if (VIR_REALLOC_N(def-hostdevs, def-nhostdevs+1) 0) {
-virDomainHostdevDefFree(dev);
+if (VIR_REALLOC_N(def-hostdevs, def-nhostdevs+1) 0)
goto error;
-}
def-hostdevs[def-nhostdevs++] = dev;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 16/23] Fix leak on OOM in qemuMonitorCommonTestNew
From: Daniel P. Berrange berra...@redhat.com Don't leak the path string in qemuMonitorCommonTestNew if an OOM occurs. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/qemumonitortestutils.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 486d72f..763102c 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -793,6 +793,7 @@ qemuMonitorCommonTestNew(virDomainXMLOptionPtr xmlopt, src-type = VIR_DOMAIN_CHR_TYPE_UNIX; src-data.nix.path = (char *)path; src-data.nix.listen = false; +path = NULL; if (virNetSocketListen(test-server, 1) 0) goto error; @@ -801,6 +802,7 @@ cleanup: return test; error: +VIR_FREE(path); VIR_FREE(tmpdir_template); qemuMonitorTestFree(test); test = NULL; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 23/23] Avoid use of uninitialized data in virnetmessagetest
From: Daniel P. Berrange berra...@redhat.com
If an error occurs in virnetmessagetest it was possible it
would free uninitialized data.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tests/virnetmessagetest.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/virnetmessagetest.c b/tests/virnetmessagetest.c
index 3c9bead..eabc609 100644
--- a/tests/virnetmessagetest.c
+++ b/tests/virnetmessagetest.c
@@ -327,6 +327,8 @@ static int testMessagePayloadDecode(const void *args
ATTRIBUTE_UNUSED)
};
int ret = -1;
+memset(err, 0, sizeof(err));
+
if (!msg)
return -1;
@@ -334,7 +336,6 @@ static int testMessagePayloadDecode(const void *args
ATTRIBUTE_UNUSED)
if (VIR_ALLOC_N(msg-buffer, msg-bufferLength) 0)
goto cleanup;
memcpy(msg-buffer, input_buffer, msg-bufferLength);
-memset(err, 0, sizeof(err));
if (virNetMessageDecodeLength(msg) 0) {
VIR_DEBUG(Failed to decode message header);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/23] Fix leak of comment string if virConfAddEntry fails on OOM
From: Daniel P. Berrange berra...@redhat.com
The code parsing comments in config files called virConfAddEntry
but did not check for failure. This caused the comment string to
leak on OOM.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virconf.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 9952f3f..e882d15 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -589,7 +589,10 @@ virConfParseComment(virConfParserCtxtPtr ctxt)
while ((ctxt-cur ctxt-end) (!IS_EOL(CUR))) NEXT;
if (VIR_STRNDUP(comm, base, ctxt-cur - base) 0)
return -1;
-virConfAddEntry(ctxt-conf, NULL, NULL, comm);
+if (virConfAddEntry(ctxt-conf, NULL, NULL, comm) == NULL) {
+VIR_FREE(comm);
+return -1;
+}
return 0;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/23] Add missing check for OOM with virVMXEscapeHexPipe
From: Daniel P. Berrange berra...@redhat.com
The virVMXFormatConfig called virVMXEscapeHexPipe but
forgot to check for OOM. This caused data to silently
be lost.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/vmx/vmx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 5c2c794..38b7cc0 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -3096,7 +3096,8 @@ virVMXFormatConfig(virVMXContext *ctx,
virDomainXMLOptionPtr xmlopt, virDomainDe
/* def:description - vmx:annotation */
if (def-description != NULL) {
-annotation = virVMXEscapeHexPipe(def-description);
+if (!(annotation = virVMXEscapeHexPipe(def-description)))
+goto cleanup;
virBufferAsprintf(buffer, annotation = \%s\\n, annotation);
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 19/23] Avoid crash on OOM in virbuftest
From: Daniel P. Berrange berra...@redhat.com
The virbuftest code did not check virBufferError before
accessing the buffer contents, resulting in a crash on
OOM conditions.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tests/virbuftest.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/tests/virbuftest.c b/tests/virbuftest.c
index febe6e4..a6dcae6 100644
--- a/tests/virbuftest.c
+++ b/tests/virbuftest.c
@@ -108,6 +108,10 @@ static int testBufAutoIndent(const void *data
ATTRIBUTE_UNUSED)
}
virBufferAdjustIndent(buf, 2);
virBufferAddLit(buf, 1);
+if (virBufferError(buf)) {
+TEST_ERROR(Buffer had error);
+return -1;
+}
if (STRNEQ(virBufferCurrentContent(buf), 1)) {
TEST_ERROR(Wrong content);
ret = -1;
@@ -134,6 +138,11 @@ static int testBufAutoIndent(const void *data
ATTRIBUTE_UNUSED)
virBufferEscapeShell(buf, 11);
virBufferAddChar(buf, '\n');
+if (virBufferError(buf)) {
+TEST_ERROR(Buffer had error);
+return -1;
+}
+
result = virBufferContentAndReset(buf);
if (!result || STRNEQ(result, expected)) {
virtTestDifference(stderr, expected, result);
@@ -166,6 +175,11 @@ static int testBufTrim(const void *data ATTRIBUTE_UNUSED)
virBufferTrim(buf, b,,, 1);
virBufferTrim(buf, ,, -1);
+if (virBufferError(buf)) {
+TEST_ERROR(Buffer had error);
+return -1;
+}
+
result = virBufferContentAndReset(buf);
if (!result || STRNEQ(result, expected)) {
virtTestDifference(stderr, expected, result);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/23] Fix leak of parser state in virJSONValueFromString
From: Daniel P. Berrange berra...@redhat.com
If OOM or another error occurs in virJSONValueFromString the
parser state object will be leaked.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virjson.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 8918bc7..2bb7324 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1014,6 +1014,7 @@ cleanup:
for (i = 0; i parser.nstate; i++) {
VIR_FREE(parser.state[i].key);
}
+VIR_FREE(parser.state);
}
VIR_DEBUG(result=%p, parser.head);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 20/23] Avoid crash on OOM in virlockspacetest
From: Daniel P. Berrange berra...@redhat.com The virlockspacetest.c did not check for failure to create a lockspace, causing a crash on OOM Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/virlockspacetest.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/virlockspacetest.c b/tests/virlockspacetest.c index b659f61..1985a4a 100644 --- a/tests/virlockspacetest.c +++ b/tests/virlockspacetest.c @@ -44,7 +44,8 @@ static int testLockSpaceCreate(const void *args ATTRIBUTE_UNUSED) rmdir(LOCKSPACE_DIR); -lockspace = virLockSpaceNew(LOCKSPACE_DIR); +if (!(lockspace = virLockSpaceNew(LOCKSPACE_DIR))) +goto cleanup; if (!virFileIsDir(LOCKSPACE_DIR)) goto cleanup; @@ -65,7 +66,8 @@ static int testLockSpaceResourceLifecycle(const void *args ATTRIBUTE_UNUSED) rmdir(LOCKSPACE_DIR); -lockspace = virLockSpaceNew(LOCKSPACE_DIR); +if (!(lockspace = virLockSpaceNew(LOCKSPACE_DIR))) +goto cleanup; if (!virFileIsDir(LOCKSPACE_DIR)) goto cleanup; @@ -98,7 +100,8 @@ static int testLockSpaceResourceLockExcl(const void *args ATTRIBUTE_UNUSED) rmdir(LOCKSPACE_DIR); -lockspace = virLockSpaceNew(LOCKSPACE_DIR); +if (!(lockspace = virLockSpaceNew(LOCKSPACE_DIR))) +goto cleanup; if (!virFileIsDir(LOCKSPACE_DIR)) goto cleanup; @@ -143,7 +146,8 @@ static int testLockSpaceResourceLockExclAuto(const void *args ATTRIBUTE_UNUSED) rmdir(LOCKSPACE_DIR); -lockspace = virLockSpaceNew(LOCKSPACE_DIR); +if (!(lockspace = virLockSpaceNew(LOCKSPACE_DIR))) +goto cleanup; if (!virFileIsDir(LOCKSPACE_DIR)) goto cleanup; @@ -180,7 +184,8 @@ static int testLockSpaceResourceLockShr(const void *args ATTRIBUTE_UNUSED) rmdir(LOCKSPACE_DIR); -lockspace = virLockSpaceNew(LOCKSPACE_DIR); +if (!(lockspace = virLockSpaceNew(LOCKSPACE_DIR))) +goto cleanup; if (!virFileIsDir(LOCKSPACE_DIR)) goto cleanup; @@ -233,7 +238,8 @@ static int testLockSpaceResourceLockShrAuto(const void *args ATTRIBUTE_UNUSED) rmdir(LOCKSPACE_DIR); -lockspace = virLockSpaceNew(LOCKSPACE_DIR); +if (!(lockspace = virLockSpaceNew(LOCKSPACE_DIR))) +goto cleanup; if (!virFileIsDir(LOCKSPACE_DIR)) goto cleanup; @@ -292,7 +298,8 @@ static int testLockSpaceResourceLockPath(const void *args ATTRIBUTE_UNUSED) rmdir(LOCKSPACE_DIR); -lockspace = virLockSpaceNew(NULL); +if (!(lockspace = virLockSpaceNew(NULL))) +goto cleanup; if (mkdir(LOCKSPACE_DIR, 0700) 0) goto cleanup; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/23] Don't clobber return value in virInterfaceDefParseProtoIPv6
From: Daniel P. Berrange berra...@redhat.com
Several places in virInterfaceDefParseProtoIPv6 clobber the
default 'ret' return value. So when jumping to cleanup on
error, 'ret' may mistakenly be set to 0 instead of -1. This
caused failure to report OOM errors, meaning data was silently
lost during parsing.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/conf/interface_conf.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 19d0327..79aab6e 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -309,9 +309,8 @@ virInterfaceDefParseProtoIPv4(virInterfaceProtocolDefPtr
def,
dhcp = virXPathNode(./dhcp, ctxt);
if (dhcp != NULL) {
-ret = virInterfaceDefParseDhcp(def, dhcp, ctxt);
-if (ret != 0)
- return ret;
+if (virInterfaceDefParseDhcp(def, dhcp, ctxt) 0)
+return -1;
}
nIpNodes = virXPathNodeSet(./ip, ctxt, ipNodes);
@@ -332,8 +331,7 @@ virInterfaceDefParseProtoIPv4(virInterfaceProtocolDefPtr
def,
goto error;
ctxt-node = ipNodes[i];
-ret = virInterfaceDefParseIp(ip, ctxt);
-if (ret != 0) {
+if (virInterfaceDefParseIp(ip, ctxt) 0) {
virInterfaceIpDefFree(ip);
goto error;
}
@@ -365,9 +363,8 @@ virInterfaceDefParseProtoIPv6(virInterfaceProtocolDefPtr
def,
dhcp = virXPathNode(./dhcp, ctxt);
if (dhcp != NULL) {
-ret = virInterfaceDefParseDhcp(def, dhcp, ctxt);
-if (ret != 0)
- return ret;
+if (virInterfaceDefParseDhcp(def, dhcp, ctxt) 0)
+return -1;
}
nIpNodes = virXPathNodeSet(./ip, ctxt, ipNodes);
@@ -388,8 +385,7 @@ virInterfaceDefParseProtoIPv6(virInterfaceProtocolDefPtr
def,
goto error;
ctxt-node = ipNodes[i];
-ret = virInterfaceDefParseIp(ip, ctxt);
-if (ret != 0) {
+if (virInterfaceDefParseIp(ip, ctxt) 0) {
virInterfaceIpDefFree(ip);
goto error;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 21/23] Avoid crash on OOM in virportallocatortest
From: Daniel P. Berrange berra...@redhat.com
The virportallocatortest did not check if the object
allocation failed in all cases. This lead to a crash
on OOM in the testsuite
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tests/virportallocatortest.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
index 1a0cdfa..615fa15 100644
--- a/tests/virportallocatortest.c
+++ b/tests/virportallocatortest.c
@@ -67,6 +67,9 @@ static int testAllocAll(const void *args ATTRIBUTE_UNUSED)
int ret = -1;
unsigned short p1, p2, p3, p4, p5, p6, p7;
+if (!alloc)
+return -1;
+
if (virPortAllocatorAcquire(alloc, p1) 0)
goto cleanup;
if (p1 != 5901) {
@@ -137,6 +140,9 @@ static int testAllocReuse(const void *args ATTRIBUTE_UNUSED)
int ret = -1;
unsigned short p1, p2, p3, p4;
+if (!alloc)
+return -1;
+
if (virPortAllocatorAcquire(alloc, p1) 0)
goto cleanup;
if (p1 != 5901) {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 22/23] Avoid crash on OOM in virnetmessagetest
From: Daniel P. Berrange berra...@redhat.com The virnetmessagetest code did not check for failure to allocate the message object. This lead to a crash on OOM in the test suite. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/virnetmessagetest.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tests/virnetmessagetest.c b/tests/virnetmessagetest.c index 1aa4c25..3c9bead 100644 --- a/tests/virnetmessagetest.c +++ b/tests/virnetmessagetest.c @@ -327,6 +327,9 @@ static int testMessagePayloadDecode(const void *args ATTRIBUTE_UNUSED) }; int ret = -1; +if (!msg) +return -1; + msg-bufferLength = 4; if (VIR_ALLOC_N(msg-buffer, msg-bufferLength) 0) goto cleanup; @@ -476,6 +479,9 @@ static int testMessagePayloadStreamEncode(const void *args ATTRIBUTE_UNUSED) }; int ret = -1; +if (!msg) +return -1; + msg-header.prog = 0x11223344; msg-header.vers = 0x01; msg-header.proc = 0x666; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/23] Fix leak of iterators in virDBusMessageIterEncode
From: Daniel P. Berrange berra...@redhat.com
If virDBusMessageIterEncode hits an OOM condition it often
leaks the memory associated with the dbus iterator object
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virdbus.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index a2c4b4e..60ff574 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -601,8 +601,10 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
goto cleanup;
if (virDBusTypeStackPush(stack, nstack,
iter, types,
- nstruct, narray) 0)
+ nstruct, narray) 0) {
+VIR_FREE(newiter);
goto cleanup;
+}
VIR_FREE(contsig);
iter = newiter;
newiter = NULL;
@@ -625,8 +627,10 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
goto cleanup;
if (virDBusTypeStackPush(stack, nstack,
iter, types,
- nstruct, narray) 0)
+ nstruct, narray) 0) {
+VIR_FREE(newiter);
goto cleanup;
+}
iter = newiter;
newiter = NULL;
types = vsig;
@@ -657,8 +661,10 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
if (virDBusTypeStackPush(stack, nstack,
iter, types,
- nstruct, narray) 0)
+ nstruct, narray) 0) {
+VIR_FREE(newiter);
goto cleanup;
+}
VIR_FREE(contsig);
iter = newiter;
newiter = NULL;
@@ -678,6 +684,18 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
ret = 0;
cleanup:
+while (nstack 0) {
+DBusMessageIter *thisiter = iter;
+VIR_DEBUG(Popping iter=%p, iter);
+if (virDBusTypeStackPop(stack, nstack, iter,
+types, nstruct, narray) 0)
+goto cleanup;
+VIR_DEBUG(Popped iter=%p, iter);
+
+if (thisiter != rootiter)
+VIR_FREE(thisiter);
+}
+
virDBusTypeStackFree(stack, nstack);
VIR_FREE(contsig);
VIR_FREE(newiter);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/23] Fix crash on OOM parsing storage pool XML
From: Daniel P. Berrange berra...@redhat.com
The virStoragePoolDefParseSource method would set def-nhosts
before allocating def-hosts. If the allocation failed due to
OOM, the cleanup code would crash accessing out of bounds.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/conf/storage_conf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 002663f..975e662 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -594,11 +594,11 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
if ((n = virXPathNodeSet(./host, ctxt, nodeset)) 0)
goto cleanup;
-source-nhost = n;
-if (source-nhost) {
-if (VIR_ALLOC_N(source-hosts, source-nhost) 0)
+if (n) {
+if (VIR_ALLOC_N(source-hosts, n) 0)
goto cleanup;
+source-nhost = n;
for (i = 0; i source-nhost; i++) {
name = virXMLPropString(nodeset[i], name);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 15/23] Don't ignore errors parsing nwfilter rules
From: Daniel P. Berrange berra...@redhat.com
For inexplicable reasons, the nwfilter XML parser is intentionally
ignoring errors that arise during parsing. As well as meaning that
users don't get any feedback on their XML mistakes, this will lead
it to silently drop data in OOM conditions.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/conf/nwfilter_conf.c| 16
tests/nwfilterxml2xmltest.c | 14 ++
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 00e74eb..3456b77 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2369,9 +2369,7 @@ virNWFilterRuleParse(xmlNodePtr node)
if (virNWFilterRuleDetailsParse(cur,
ret,
virAttr[i].att) 0) {
-/* we ignore malformed rules
- goto err_exit;
-*/
+goto err_exit;
}
break;
}
@@ -2573,11 +2571,13 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) {
if (VIR_ALLOC(entry) 0)
goto cleanup;
-/* ignore malformed rule and include elements */
-if (xmlStrEqual(curr-name, BAD_CAST rule))
-entry-rule = virNWFilterRuleParse(curr);
-else if (xmlStrEqual(curr-name, BAD_CAST filterref))
-entry-include = virNWFilterIncludeParse(curr);
+if (xmlStrEqual(curr-name, BAD_CAST rule)) {
+if (!(entry-rule = virNWFilterRuleParse(curr)))
+goto cleanup;
+} else if (xmlStrEqual(curr-name, BAD_CAST filterref)) {
+if (!(entry-include = virNWFilterIncludeParse(curr)))
+goto cleanup;
+}
if (entry-rule || entry-include) {
if (VIR_REALLOC_N(ret-filterEntries, ret-nentries+1) 0) {
diff --git a/tests/nwfilterxml2xmltest.c b/tests/nwfilterxml2xmltest.c
index 5476284..84e61da 100644
--- a/tests/nwfilterxml2xmltest.c
+++ b/tests/nwfilterxml2xmltest.c
@@ -36,15 +36,12 @@ testCompareXMLToXMLFiles(const char *inxml, const char
*outxml,
virResetLastError();
-if (!(dev = virNWFilterDefParseString(NULL, inXmlData)))
+if (!(dev = virNWFilterDefParseString(NULL, inXmlData))) {
+if (expect_error) {
+virResetLastError();
+goto done;
+}
goto fail;
-
-if (!!virGetLastError() != expect_error)
-goto fail;
-
-if (expect_error) {
-/* need to suppress the errors */
-virResetLastError();
}
if (!(actual = virNWFilterDefFormat(dev)))
@@ -55,6 +52,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char
*outxml,
goto fail;
}
+ done:
ret = 0;
fail:
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh-domain: Add a missing check and fix leak in cmdScreenshot
---
tools/virsh-domain.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 856e888..7fe42df 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4610,7 +4610,8 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, name)))
return false;
-st = virStreamNew(ctl-conn, 0);
+if (!(st = virStreamNew(ctl-conn, 0)))
+goto cleanup;
mime = virDomainScreenshot(dom, st, screen, flags);
if (!mime) {
@@ -4620,7 +4621,7 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
if (!file) {
if (!(file=vshGenFileName(ctl, dom, mime)))
-return false;
+goto cleanup;
generated = true;
}
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] LXC: Detect fs support. Mount only supported fs in containers
On Wed, Sep 25, 2013 at 5:49 AM, Bogdan Purcareata
bogdan.purcare...@freescale.com wrote:
Some filesystems - specifically securityfs - are not supported in
all systems running libvirt containers. When starting a container,
mount only the filesystems that are supported on the host. Detection
of filesystem support is done at runtime.
Might be worth noting this behavior is since 6807238d87fd93dee30
Signed-off-by: Bogdan Purcareata bogdan.purcare...@freescale.com
---
src/lxc/lxc_container.c | 61
+
1 file changed, 61 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index c60f5d8..eff9a24 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -509,6 +509,61 @@ static int lxcContainerChildMountSort(const void *a,
const void *b)
# define MS_SLAVE(119)
#endif
+/*
+ * This function attempts to detect kernel support
+ * for a specific filesystem type. This is done by
+ * inspecting /proc/filesystems.
+ */
+static int lxcCheckFSSupport(const char *fs_type)
+{
+FILE *fp = NULL;
+int ret = -1;
So your error case here is -1, which is good which we typically use.
+const char *fslist = /proc/filesystems;
+char *line = NULL;
+const char *type;
+
+if (!fs_type)
+return 1;
but here its 1. You appear to use 1 as the success case below so
this conflicts.
+
+VIR_DEBUG(Checking kernel support for %s, fs_type);
+
+VIR_DEBUG(Open %s, fslist);
Can we combine these two debugs?
+if (!(fp = fopen(fslist, r))) {
+if (errno == ENOENT)
It seems like you had another line after this if check that went away
since the line below is not indented properly. Since we always want to
print the error message you probably want to drop this if check
+
+virReportSystemError(errno,
+ _(Unable to read %s),
+ fslist);
+goto cleanup;
+}
+
+while (!feof(fp)) {
+size_t n;
+VIR_FREE(line);
+if (getline(line, n, fp) = 0) {
+if (feof(fp))
+break;
This could really be optimized with:
while (getline(line, n, fp) 0) {
.do work here
}
if (ferror(fp)) {
handle error ...
}
And we wouldn't have to have nested error checking.
+
+goto cleanup;
+}
+
+type = strstr(line, fs_type);
So I dislike using strstr() here because it'll be a greedy match. e.g.
should you check for fuse, it will also match fuseblk and
fusectl. Or check for nfs and it'll match on nfs4.
+if (type) {
+ret = 1;
So here's the success case that it was found and its set to 1.
+goto cleanup;
+}
+}
+
+VIR_DEBUG(No kernel support for %s, fs_type);
+
+ret = 0;
+
+cleanup:
+VIR_FREE(line);
+VIR_FORCE_FCLOSE(fp);
+return ret;
+}
+
static int lxcContainerGetSubtree(const char *prefix,
char ***mountsret,
size_t *nmountsret)
@@ -855,17 +910,23 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
for (i = 0; i ARRAY_CARDINALITY(lxcBasicMounts); i++) {
virLXCBasicMountInfo const *mnt = lxcBasicMounts[i];
const char *srcpath = NULL;
+ const char *dstpath = NULL;
VIR_DEBUG(Processing %s - %s,
mnt-src, mnt-dst);
srcpath = mnt-src;
+ dstpath = mnt-dst;
/* Skip if mount doesn't exist in source */
if ((srcpath[0] == '/')
(access(srcpath, R_OK) 0))
continue;
+ if ((access(dstpath, R_OK) 0) || /* mount is not present on host */
So I understand you are doing this for securityfs. But there's other
cases when the filesystem might not be mounted on the host but we want
to mount it in the container so I could see this check wrecking havoc
with that. I could also be wrong.
+ (!lxcCheckFSSupport(mnt-type))) /* no fs support in kernel */
+ continue;
+
#if WITH_SELINUX
if (STREQ(mnt-src, SELINUX_MOUNT)
(!is_selinux_enabled() || userns_enabled))
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Hopefully some of the comments above help. Thanks for fixing this
since I too had a kernel running without securityfs and hit this.
--
Doug Goldstein
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] conf: Refactor storing and usage of feature flags
On 09/24/2013 10:43 AM, Peter Krempa wrote:
To allow a finer control for some future flags, refactor the code to
store them in an array instead of a bitmap.
---
src/conf/domain_conf.c | 166
++---
src/conf/domain_conf.h | 2 +-
src/libxl/libxl_conf.c | 9 ++-
src/lxc/lxc_container.c| 6 +-
src/qemu/qemu_command.c| 16 ++---
src/vbox/vbox_tmpl.c | 45 ++--
src/xenapi/xenapi_driver.c | 10 +--
src/xenapi/xenapi_utils.c | 22 +++---
src/xenxs/xen_sxpr.c | 20 +++---
src/xenxs/xen_xm.c | 30
10 files changed, 191 insertions(+), 135 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8953579..e1a1d6d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11367,29 +11367,40 @@ virDomainDefParseXML(xmlDocPtr xml,
int val = virDomainFeatureTypeFromString((const char
*)nodes[i]-name);
if (val 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(unexpected feature %s),
- nodes[i]-name);
+ _(unexpected feature '%s'), nodes[i]-name);
Unrelated change.
goto error;
}
-def-features |= (1 val);
-if (val == VIR_DOMAIN_FEATURE_APIC) {
-tmp = virXPathString(string(./features/apic/@eoi), ctxt);
-if (tmp) {
+
+switch ((enum virDomainFeature) val) {
+case VIR_DOMAIN_FEATURE_APIC:
+if ((tmp = virXPathString(string(./features/apic/@eoi),
ctxt))) {
int eoi;
if ((eoi = virDomainFeatureStateTypeFromString(tmp)) = 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(unknown value for attribute eoi: %s),
+ _(unknown value for attribute eoi:
'%s'),
Another unrelated change.
tmp);
goto error;
}
-def-apic_eoi = eoi;
+ def-apic_eoi = eoi;
Indentantion is off.
VIR_FREE(tmp);
}
+/* fallthrough */
+case VIR_DOMAIN_FEATURE_ACPI:
+case VIR_DOMAIN_FEATURE_PAE:
+case VIR_DOMAIN_FEATURE_HAP:
+case VIR_DOMAIN_FEATURE_VIRIDIAN:
+case VIR_DOMAIN_FEATURE_PRIVNET:
+case VIR_DOMAIN_FEATURE_HYPERV:
+def-features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
+break;
+
+case VIR_DOMAIN_FEATURE_LAST:
+break;
}
}
VIR_FREE(nodes);
-if (def-features (1 VIR_DOMAIN_FEATURE_HYPERV)) {
+if (def-features[VIR_DOMAIN_FEATURE_HYPERV] ==
VIR_DOMAIN_FEATURE_STATE_ON) {
int feature;
int value;
node = ctxt-node;
@@ -13361,12 +13372,16 @@
virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
{
size_t i;
-/* basic check */
-if (src-features != dst-features) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(Target domain features %d does not match source
%d),
- dst-features, src-features);
-return false;
+for (i = 0; i VIR_DOMAIN_FEATURE_LAST; i++) {
+if (src-features[i] != dst-features[i]) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(State of feature '%s' differs:
+ source: '%s', destination: '%s'),
+ virDomainFeatureTypeToString(i),
+
virDomainFeatureStateTypeToString(src-features[i]),
+
virDomainFeatureStateTypeToString(dst-features[i]));
+return false;
+}
Yay, frendlier error message!
@@ -16703,58 +16718,99 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAddLit(buf, /idmap\n);
}
+for (i = 0; i VIR_DOMAIN_FEATURE_LAST; i++) {
+if (def-features[i] != VIR_DOMAIN_FEATURE_STATE_DEFAULT)
+break;
+}
-if (def-features) {
+if (i != VIR_DOMAIN_FEATURE_LAST) {
virBufferAddLit(buf, features\n);
+
for (i = 0; i VIR_DOMAIN_FEATURE_LAST; i++) {
-if (def-features (1 i) i != VIR_DOMAIN_FEATURE_HYPERV) {
-const char *name = virDomainFeatureTypeToString(i);
-if (!name) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(unexpected feature %zu), i);
-goto error;
-}
-virBufferAsprintf(buf, %s, name);
-if (i == VIR_DOMAIN_FEATURE_APIC def-apic_eoi) {
-virBufferAsprintf(buf,
- eoi='%s',
-
Re: [libvirt] [PATCH RFC 3/3] qemu: Add support for paravirtual spinlocks in the guest
On 09/24/2013 10:43 AM, Peter Krempa wrote:
The linux kernel recently added support for paravirtual spinlock
handling to avoid performance regressions on overcomitted hosts. This
feature needs to be turned in the hypervisor so that the guest OS is
s/turned in/turned on in/
notified about the possible support.
This patch adds a new feature paravirt-spinlock to the XML and
supporting code to enable the kvm_pv_unhalt pseudoCPU feature in qemu.
I'm guessing we can't split those without compiler warnings about not all enum
values being handled in the switch.
https://bugzilla.redhat.com/show_bug.cgi?id=1008989
---
This patch is RFC as qemu didn't add this to the upstream repo yet:
Now upstream:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=f010bc6
Pull request:
http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg03385.html
Original thread:
http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg03095.html
I'm sending it to get review feedback but will push it only after qemu will
have it.
docs/formatdomain.html.in | 7 ++
docs/schemas/domaincommon.rng | 10 -
src/conf/domain_conf.c | 20 -
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c| 14
.../qemuxml2argv-pv-spinlock-disabled.args | 5 +
.../qemuxml2argv-pv-spinlock-disabled.xml | 26
++
.../qemuxml2argv-pv-spinlock-enabled.args | 5 +
.../qemuxml2argv-pv-spinlock-enabled.xml | 26
++
tests/qemuxml2argvtest.c | 2 ++
tests/qemuxml2xmltest.c| 2 ++
11 files changed, 116 insertions(+), 2 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml
@@ -16742,6 +16759,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
case VIR_DOMAIN_FEATURE_HAP:
case VIR_DOMAIN_FEATURE_VIRIDIAN:
case VIR_DOMAIN_FEATURE_PRIVNET:
+case VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK:
switch ((enum virDomainFeatureState) def-features[i]) {
case VIR_DOMAIN_FEATURE_STATE_ON:
/* output just the element */
Unlike the features above, this one can actually be turned off.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] schema: Rename option to make it reusable
On 09/24/2013 10:43 AM, Peter Krempa wrote: --- docs/schemas/domaincommon.rng | 8 1 file changed, 4 insertions(+), 4 deletions(-) ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh-domain: Add a missing check and fix leak in cmdScreenshot
On 09/25/2013 08:54 AM, Hongwei Bi wrote:
---
tools/virsh-domain.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
@@ -4620,7 +4621,7 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
if (!file) {
if (!(file=vshGenFileName(ctl, dom, mime)))
I fixed up the spacing of this = while at it.
-return false;
+goto cleanup;
ACK and pushed.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/23] Don't clobber 'ret' in LXC XML test case
On 09/25/2013 08:51 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The testCompareXMLToXMLHelper method clobbered the 'ret' variable in several places leading to a failure to report OOM errors from the test suite. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/lxcxml2xmltest.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) ACK 1-5. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Fix domdisplay when domain only uses TLS
It's possible to create a domain which will only use a TLS port
and will not have a non-TLS port set by using:
graphics type='spice' autoport='yes' defaultMode='secure'/
In such a setup, the 'graphics' node for the running domain will be:
graphics type='spice' tlsPort='5900'
autoport='yes' listen='127.0.0.1'
defaultMode='secure'
However, cmdDomDisplay loops over all the 'graphics' node, and it
ignores nodes which don't have a 'port' attribute. This means
'virsh domdisplay' will only return an empty string for domains
as the one above.
This commit looks for both 'port' and 'tlsPort' before deciding
to ignore a graphics node. It also makes sure 'port' is not printed
when it's not set.
This makes 'virsh domdisplay' return
'spice://127.0.0.1?tls-port=5900' for domains using only a TLS
port.
Signed-off-by: Christophe Fergeau cferg...@redhat.com
---
tools/virsh-domain.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 856e888..5e9d784 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9073,6 +9073,20 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
/* If there is no port number for this type, then jump to the next
* scheme */
if (tmp)
+port = 0;
+
+/* Create our XPATH lookup for TLS Port (automatically skipped
+ * for unsupported schemes */
+if (virAsprintf(xpath, xpath_fmt, scheme[iter], tlsPort) 0)
+goto cleanup;
+
+/* Attempt to get the TLS port number */
+tmp = virXPathInt(xpath, ctxt, tls_port);
+VIR_FREE(xpath);
+if (tmp)
+tls_port = 0;
+
+if (!port !tls_port)
continue;
/* Create our XPATH lookup for the current display's address */
@@ -9103,17 +9117,6 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
port -= 5900;
}
-/* Create our XPATH lookup for TLS Port (automatically skipped
- * for unsupported schemes */
-if (virAsprintf(xpath, xpath_fmt, scheme[iter], tlsPort) 0)
-goto cleanup;
-
-/* Attempt to get the TLS port number */
-tmp = virXPathInt(xpath, ctxt, tls_port);
-VIR_FREE(xpath);
-if (tmp)
-tls_port = 0;
-
/* Build up the full URI, starting with the scheme */
virBufferAsprintf(buf, %s://, scheme[iter]);
@@ -9128,7 +9131,8 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
virBufferAsprintf(buf, %s, listen_addr);
/* Add the port */
-virBufferAsprintf(buf, :%d, port);
+if (port)
+virBufferAsprintf(buf, :%d, port);
/* TLS Port */
if (tls_port) {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/23] Don't print all test suite errors to stderr in vmx2xmltest
On 09/25/2013 08:51 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The vmx2xmltest test would print all errors to stderr, which
is not helpful when running OOM tests, and differs from the
behaviour of other tests.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tests/vmx2xmltest.c | 51 +++
1 file changed, 19 insertions(+), 32 deletions(-)
@@ -203,7 +190,7 @@ mymain(void)
virResetLastError();
\
if (virtTestRun(VMware VMX-2-XML _in - _out, 1,
\
testCompareHelper, info) 0) {
\
-result = -1;
\
+ret = -1; \
}
\
Indentation of \ is now off.
ACK to 6-10
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 3/3] qemu: Add support for paravirtual spinlocks in the guest
On Tue, Sep 24, 2013 at 3:43 AM, Peter Krempa pkre...@redhat.com wrote:
The linux kernel recently added support for paravirtual spinlock
handling to avoid performance regressions on overcomitted hosts. This
feature needs to be turned in the hypervisor so that the guest OS is
notified about the possible support.
This patch adds a new feature paravirt-spinlock to the XML and
supporting code to enable the kvm_pv_unhalt pseudoCPU feature in qemu.
https://bugzilla.redhat.com/show_bug.cgi?id=1008989
---
This patch is RFC as qemu didn't add this to the upstream repo yet:
Pull request:
http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg03385.html
Original thread:
http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg03095.html
I'm sending it to get review feedback but will push it only after qemu will
have it.
docs/formatdomain.html.in | 7 ++
docs/schemas/domaincommon.rng | 10 -
src/conf/domain_conf.c | 20 -
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c| 14
.../qemuxml2argv-pv-spinlock-disabled.args | 5 +
.../qemuxml2argv-pv-spinlock-disabled.xml | 26
++
.../qemuxml2argv-pv-spinlock-enabled.args | 5 +
.../qemuxml2argv-pv-spinlock-enabled.xml | 26
++
tests/qemuxml2argvtest.c | 2 ++
tests/qemuxml2xmltest.c| 2 ++
11 files changed, 116 insertions(+), 2 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3689399..aa0076c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1182,6 +1182,7 @@
lt;vapic state='on'/gt;
lt;spinlocks state='on' retries='4096'/gt;
lt;/hypervgt;
+lt;paravirt-spinlock/gt;
lt;/featuresgt;
.../pre
@@ -1253,6 +1254,12 @@
/tr
/table
/dd
+ dtcodeparavirtual-spinlock/code/dt
You call it paravirt-spinlock everywhere else so this looks like a typo.
+ ddNotify the guest that the host supports paravirtual spinlocks
+ for example by exposing the pvticketlocks mechanism. This feature
+ can be forced of by using a state='off' attribute.
+ /dd
+
/dl
h3a name=elementsTimeTime keeping/a/h3
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 94a72f8..4c494cb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3560,7 +3560,7 @@
/define
!--
A set of optional features: PAE, APIC, ACPI,
- HyperV Enlightenment and HAP support
+ HyperV Enlightenment, paravirtual spinlocks and HAP support
--
define name=features
optional
@@ -3606,6 +3606,14 @@
empty/
/element
/optional
+ optional
+element name=paravirt-spinlock
+ optional
+ref name=featurestate/
+ /optional
+ empty/
+/element
+ /optional
/interleave
/element
/optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e1a1d6d..919adc1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -142,7 +142,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
hap,
viridian,
privnet,
- hyperv)
+ hyperv,
+ paravirt-spinlock)
VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST,
default,
@@ -11394,6 +11395,22 @@ virDomainDefParseXML(xmlDocPtr xml,
def-features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
break;
+case VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK:
+node = ctxt-node;
+ctxt-node = nodes[i];
+if ((tmp = virXPathString(string(./@state), ctxt))) {
+if ((def-features[val] =
virDomainFeatureStateTypeFromString(tmp)) == -1) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unknown state atribute '%s' of feature
'%s'),
+ tmp, virDomainFeatureTypeToString(val));
+goto error;
+}
+} else {
+def-features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
+}
+ctxt-node = node;
+break;
+
[libvirt] libvirt-glib fails to compile with CLANG compiler
When compiling libvirt-glib with CLANG, I get the following error. 16 warnings generated. CCLD libvirt-glib-1.0.la GEN LibvirtGLib-1.0.gir /usr/local/lib/libvirt.so: undefined reference to `__stack_chk_fail_local' clang: error: linker command failed with exit code 1 (use -v to see invocation) linking of temporary binary failed: Command '['/usr/local/bin/libtool', '--mode=link', '--tag=CC', '--silent', 'clang', '-o', '/usr/home/helfman/workspace/ports/devel/libvirt-glib/work/libvirt-glib-0.1.7/libvirt-glib/tmp-introspectOO6vqG/LibvirtGLib-1.0', '-export-dynamic', '-O2', '-pipe', '-fno-strict-aliasing', '-L/usr/local/lib', '-fstack-protector', '/usr/home/helfman/workspace/ports/devel/libvirt-glib/work/libvirt-glib-0.1.7/libvirt-glib/tmp-introspectOO6vqG/LibvirtGLib-1.0.o', '-L.', './libvirt-glib-1.0.la', '-lgio-2.0', '-lgobject-2.0', '-Wl,--export-dynamic', '-lgmodule-2.0', '-lgthread-2.0', '-pthread', '-L/usr/local/lib', '-lglib-2.0', '-lintl']' returned non-zero exit status 1 gmake[2]: *** [LibvirtGLib-1.0.gir] Error 1 gmake[2]: Leaving directory `/usr/home/helfman/workspace/ports/devel/libvirt-glib/work/libvirt-glib-0.1.7/libvirt-glib' gmake[1]: *** [all-recursive] Error 1 gmake[1]: Leaving directory `/usr/home/helfman/workspace/ports/devel/libvirt-glib/work/libvirt-glib-0.1.7' gmake: *** [all] Error 2 *** [do-build] Error code 1 -jgh -- Jason Helfman | FreeBSD Committer j...@freebsd.org | http://people.freebsd.org/~jgh | The Power to Serve -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/23] Fix leak of iterators in virDBusMessageIterEncode
On 09/25/2013 08:51 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
If virDBusMessageIterEncode hits an OOM condition it often
leaks the memory associated with the dbus iterator object
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
cleanup:
+while (nstack 0) {
+DBusMessageIter *thisiter = iter;
+VIR_DEBUG(Popping iter=%p, iter);
+if (virDBusTypeStackPop(stack, nstack, iter,
+types, nstruct, narray) 0)
+goto cleanup;
I'm worried that this will infloop. What guarantee do we have that
nstack reduces when virDBusTypeStackPop fails?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/23] Don't ignore errors parsing nwfilter rules
On 09/25/2013 08:51 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com For inexplicable reasons, the nwfilter XML parser is intentionally ignoring errors that arise during parsing. As well as meaning that users don't get any feedback on their XML mistakes, this will lead it to silently drop data in OOM conditions. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/conf/nwfilter_conf.c| 16 tests/nwfilterxml2xmltest.c | 14 ++ 2 files changed, 14 insertions(+), 16 deletions(-) ACK 12-15. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 20/23] Avoid crash on OOM in virlockspacetest
On 09/25/2013 08:51 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The virlockspacetest.c did not check for failure to create a lockspace, causing a crash on OOM Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/virlockspacetest.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) ACK 16-20 -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 23/23] Avoid use of uninitialized data in virnetmessagetest
On 09/25/2013 08:51 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com If an error occurs in virnetmessagetest it was possible it would free uninitialized data. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tests/virnetmessagetest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) ACK 21-23 Of the series, 11 was the only patch I question. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/23] Fix leak of iterators in virDBusMessageIterEncode
On Wed, Sep 25, 2013 at 10:45:00AM -0600, Eric Blake wrote:
On 09/25/2013 08:51 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
If virDBusMessageIterEncode hits an OOM condition it often
leaks the memory associated with the dbus iterator object
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
cleanup:
+while (nstack 0) {
+DBusMessageIter *thisiter = iter;
+VIR_DEBUG(Popping iter=%p, iter);
+if (virDBusTypeStackPop(stack, nstack, iter,
+types, nstruct, narray) 0)
+goto cleanup;
I'm worried that this will infloop. What guarantee do we have that
nstack reduces when virDBusTypeStackPop fails?
It can only return -1, if nstack == 0, so I think we're fine there
static int virDBusTypeStackPop(virDBusTypeStack **stack,
size_t *nstack,
DBusMessageIter **iter,
const char **types,
size_t *nstruct,
size_t *narray)
{
if (*nstack == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(DBus type stack is empty));
return -1;
}
*iter = (*stack)[(*nstack) - 1].iter;
*types = (*stack)[(*nstack) - 1].types;
*nstruct = (*stack)[(*nstack) - 1].nstruct;
*narray = (*stack)[(*nstack) - 1].narray;
VIR_DEBUG(Popped '%s', *types);
VIR_SHRINK_N(*stack, *nstack, 1);
return 0;
}
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/23] Fix leak of iterators in virDBusMessageIterEncode
On 09/25/2013 10:54 AM, Daniel P. Berrange wrote:
On Wed, Sep 25, 2013 at 10:45:00AM -0600, Eric Blake wrote:
On 09/25/2013 08:51 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
If virDBusMessageIterEncode hits an OOM condition it often
leaks the memory associated with the dbus iterator object
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
cleanup:
+while (nstack 0) {
+DBusMessageIter *thisiter = iter;
+VIR_DEBUG(Popping iter=%p, iter);
+if (virDBusTypeStackPop(stack, nstack, iter,
+types, nstruct, narray) 0)
+goto cleanup;
I'm worried that this will infloop. What guarantee do we have that
nstack reduces when virDBusTypeStackPop fails?
It can only return -1, if nstack == 0, so I think we're fine there
But you already checked that nstack was 0. So write it:
ignore_value(virDBusTypeStackPop(...));
rather than the confusing dead-code backwards goto.
ACK with that change.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Only distribute ChangeLog in -devel package
On 09/04/2013 11:18 AM, Eric Blake wrote: On 09/04/2013 09:11 AM, Daniel P. Berrange wrote: client: COPYING COPYING.LESSER daemon: none other[*]: AUTHORS ChangeLog.gz NEWS README TODO where I have yet another question: should the other package be libvirt-docs or libvirt-devel? Your patch put them in devel, but I think docs might be a better fit. Oh, I didn't realize we had a -docs package. I just looked at -devel and saw the website docs and didn't look further. It seems we're duplicating website in both -docs and -devel which is bad. We should remove those website docs from -devel and have all docs related stuff in -docs Duplication is indeed bad. But notice this guideline: https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Documentation Any relevant documentation included in the source distribution should be included in the package as %doc. Irrelevant documentation include build instructions, the omnipresent INSTALL file containing generic build instructions, for example, and documentation for non-Linux systems, e.g. README.MSDOS. Pay also attention about which subpackage you include documentation in, for example API documentation belongs in the -devel subpackage, not the main one. Or if there's a lot of documentation, consider putting it into a subpackage. In this case, it is recommended to use *-doc as the subpackage name, and Documentation as the value of the Group tag. I'm wondering if the API docs should live in -devel, and all other files in -docs; or we could just have ALL docs live in -docs, as well as make -devel depend on -docs. Currently all the API files are in -docs, with -devel having a dep on -docs, so I'll leave it that way. Upcoming patch uses danpb's suggesting of COPYING* in -client and everything else (AUTHORS, ChangeLog, etc). in -docs Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] Remove existing OOM test impl
From: Daniel P. Berrange berra...@redhat.com
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tests/testutils.c | 188 +++---
1 file changed, 22 insertions(+), 166 deletions(-)
diff --git a/tests/testutils.c b/tests/testutils.c
index 45882c5..da69d53 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -47,10 +47,6 @@
#include virprocess.h
#include virstring.h
-#if TEST_OOM_TRACE
-# include execinfo.h
-#endif
-
#ifdef HAVE_PATHS_H
# include paths.h
#endif
@@ -68,7 +64,6 @@ static unsigned int testDebug = -1;
static unsigned int testVerbose = -1;
static unsigned int testExpensive = -1;
-static unsigned int testOOM = 0;
static size_t testCounter = 0;
static size_t testStart = 0;
static size_t testEnd = 0;
@@ -151,10 +146,8 @@ virtTestRun(const char *title, int nloops, int
(*body)(const void *data), const
testCounter testEnd))
return 0;
-if (testOOM 2) {
-if (virTestGetVerbose())
-fprintf(stderr, %2zu) %-65s ... , testCounter, title);
-}
+if (virTestGetVerbose())
+fprintf(stderr, %2zu) %-65s ... , testCounter, title);
if (nloops 1 (VIR_ALLOC_N(ts, nloops) 0))
return -1;
@@ -182,30 +175,28 @@ virtTestRun(const char *title, int nloops, int
(*body)(const void *data), const
ts[i] = DIFF_MSEC(after, before);
}
}
-if (testOOM 2) {
-if (virTestGetVerbose()) {
-if (ret == 0 ts)
-fprintf(stderr, OK [%.5f ms]\n,
-virtTestCountAverage(ts, nloops));
-else if (ret == 0)
-fprintf(stderr, OK\n);
-else if (ret == EXIT_AM_SKIP)
-fprintf(stderr, SKIP\n);
-else
-fprintf(stderr, FAILED\n);
-} else {
-if (testCounter != 1
-!((testCounter-1) % 40)) {
-fprintf(stderr, %-3zu\n, (testCounter-1));
-fprintf(stderr, );
+if (virTestGetVerbose()) {
+if (ret == 0 ts)
+fprintf(stderr, OK [%.5f ms]\n,
+virtTestCountAverage(ts, nloops));
+else if (ret == 0)
+fprintf(stderr, OK\n);
+else if (ret == EXIT_AM_SKIP)
+fprintf(stderr, SKIP\n);
+else
+fprintf(stderr, FAILED\n);
+} else {
+if (testCounter != 1
+!((testCounter-1) % 40)) {
+fprintf(stderr, %-3zu\n, (testCounter-1));
+fprintf(stderr, );
}
-if (ret == 0)
+if (ret == 0)
fprintf(stderr, .);
-else if (ret == EXIT_AM_SKIP)
-fprintf(stderr, _);
-else
-fprintf(stderr, !);
-}
+else if (ret == EXIT_AM_SKIP)
+fprintf(stderr, _);
+else
+fprintf(stderr, !);
}
VIR_FREE(ts);
@@ -539,27 +530,6 @@ virtTestLogContentAndReset(void)
return ret;
}
-#if TEST_OOM_TRACE
-static void
-virtTestErrorHook(int n, void *data ATTRIBUTE_UNUSED)
-{
-void *trace[30];
-int ntrace = ARRAY_CARDINALITY(trace);
-size_t i;
-char **symbols = NULL;
-
-ntrace = backtrace(trace, ntrace);
-symbols = backtrace_symbols(trace, ntrace);
-if (symbols) {
-fprintf(stderr, Failing allocation %d at:\n, n);
-for (i = 0; i ntrace; i++) {
-if (symbols[i])
-fprintf(stderr, TRACE: %s\n, symbols[i]);
-}
-VIR_FREE(symbols);
-}
-}
-#endif
static unsigned int
virTestGetFlag(const char *name) {
@@ -603,15 +573,6 @@ int virtTestMain(int argc,
int ret;
bool abs_srcdir_cleanup = false;
char *testRange = NULL;
-#if TEST_OOM
-int approxAlloc = 0;
-int n;
-char *oomStr = NULL;
-int oomCount;
-int mp = 0;
-pid_t *workers;
-int worker = 0;
-#endif
abs_srcdir = getenv(abs_srcdir);
if (!abs_srcdir) {
@@ -672,112 +633,7 @@ int virtTestMain(int argc,
}
}
-#if TEST_OOM
-if ((oomStr = getenv(VIR_TEST_OOM)) != NULL) {
-if (virStrToLong_i(oomStr, NULL, 10, oomCount) 0)
-oomCount = 0;
-
-if (oomCount 0)
-oomCount = 0;
-if (oomCount)
-testOOM = 1;
-}
-
-if (getenv(VIR_TEST_MP) != NULL) {
-mp = sysconf(_SC_NPROCESSORS_ONLN);
-fprintf(stderr, Using %d worker processes\n, mp);
-if (VIR_ALLOC_N(workers, mp) 0) {
-ret = EXIT_FAILURE;
-goto cleanup;
-}
-}
-
-/* Run once to prime any static allocations ensure it passes */
ret = (func)();
-if (ret != EXIT_SUCCESS)
-goto cleanup;
-
-# if TEST_OOM_TRACE
-if (virTestGetDebug())
-virAllocTestHook(virtTestErrorHook, NULL);
-# endif
-
-if (testOOM) {
-/* Makes next test runs quiet... */
-testOOM++;
-
[libvirt] [PATCH 0/4] Rewrite the test OOM checking code
From: Daniel P. Berrange berra...@redhat.com This rewrites the OOM checking code to be much much much more scalable. Instead of looping running 'main' multiple times, we loop running 'virtTestRun' multiple times. Although the overall number of mallocs to be checked is basically the same, checking them in small blocks is a massive efficiency win, because the complexity is 'n * (n + 1) / 1'. Although I've fixed many issues, there are still some big problems remaining - The APIs in virxml.h don't let the caller distinguish between attribute not found and other errors. So anywhere we have an attribute that is optional, we are failing to diagnose report OOM correctly (or indeed other errors we might get from libxml) - It is desirable to validate that we get VIR_ERR_NO_MEMORY for each failure, but there are times where we won't get this. In particular with any of the APIs which have delayed error reporting such as virCommand and virBuffer. You can see an OOM from those APIs, but there's a window between that occurring being reported, where another error may be reported. - We should not count allocs in virerror.c or virlog.c APIs when doing OOM testing, since they're intentionally non-fatal Daniel P. Berrange (4): Don't clobber 'ret' variable in testCompareXMLToXMLHelper Remove existing OOM test impl Remove test case average timing Introduce new OOM testing support tests/commandtest.c | 2 +- tests/cputest.c | 5 +- tests/domainsnapshotxml2xmltest.c | 2 +- tests/esxutilstest.c | 2 +- tests/fchosttest.c| 10 +- tests/fdstreamtest.c | 8 +- tests/interfacexml2xmltest.c | 6 +- tests/jsontest.c | 2 +- tests/libvirtdconftest.c | 2 +- tests/lxcxml2xmltest.c| 4 +- tests/metadatatest.c | 6 +- tests/networkxml2conftest.c | 2 +- tests/networkxml2xmltest.c| 2 +- tests/networkxml2xmlupdatetest.c | 2 +- tests/nodedevxml2xmltest.c| 6 +- tests/nodeinfotest.c | 2 +- tests/nwfilterxml2xmltest.c | 2 +- tests/openvzutilstest.c | 2 +- tests/qemuagenttest.c | 4 +- tests/qemuargv2xmltest.c | 18 +- tests/qemuhelptest.c | 2 +- tests/qemuhotplugtest.c | 2 +- tests/qemumonitorjsontest.c | 8 +- tests/qemumonitortest.c | 2 +- tests/qemuxml2argvtest.c | 14 +- tests/qemuxml2xmltest.c | 25 +-- tests/qemuxmlnstest.c | 20 +- tests/securityselinuxlabeltest.c | 6 +- tests/securityselinuxtest.c | 2 +- tests/sexpr2xmltest.c | 2 +- tests/sockettest.c| 16 +- tests/statstest.c | 2 +- tests/storagepoolxml2xmltest.c| 6 +- tests/storagevolxml2argvtest.c| 2 +- tests/storagevolxml2xmltest.c | 14 +- tests/sysinfotest.c | 2 +- tests/testutils.c | 386 -- tests/testutils.h | 3 +- tests/utiltest.c | 4 +- tests/viratomictest.c | 4 +- tests/virauthconfigtest.c | 2 +- tests/virbitmaptest.c | 18 +- tests/virbuftest.c| 2 +- tests/vircgrouptest.c | 22 +-- tests/virdbustest.c | 10 +- tests/virdrivermoduletest.c | 2 +- tests/virendiantest.c | 4 +- tests/virhashtest.c | 2 +- tests/viridentitytest.c | 4 +- tests/virkeycodetest.c| 4 +- tests/virkeyfiletest.c| 2 +- tests/virlockspacetest.c | 14 +- tests/virnetmessagetest.c | 10 +- tests/virnetserverclienttest.c| 2 +- tests/virnetsockettest.c | 30 +-- tests/virnettlscontexttest.c | 2 +- tests/virnettlssessiontest.c | 4 +- tests/virportallocatortest.c | 4 +- tests/virshtest.c | 36 ++-- tests/virstoragetest.c| 2 +- tests/virstringtest.c | 8 +- tests/virsystemdtest.c| 10 +- tests/virtimetest.c | 2 +- tests/viruritest.c| 2 +- tests/vmwarevertest.c | 2 +- tests/vmx2xmltest.c | 2 +- tests/xencapstest.c | 22 +-- tests/xmconfigtest.c | 4 +- tests/xml2sexprtest.c | 2 +- tests/xml2vmxtest.c | 2 +- 70 files changed, 436 insertions(+), 407 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Don't clobber 'ret' variable in testCompareXMLToXMLHelper
From: Daniel P. Berrange berra...@redhat.com
The qemuxml2xmltest.c function testCompareXMLToXMLHelper would
clobber the 'ret' variable causing it to mis-diagnose OOM
errors.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tests/qemuxml2xmltest.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index c661573..9fd3ada 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -84,16 +84,19 @@ testCompareXMLToXMLHelper(const void *data)
abs_srcdir, info-name) 0)
goto cleanup;
-if (info-when WHEN_INACTIVE) {
-ret = testCompareXMLToXMLFiles(xml_in,
- info-different ? xml_out : xml_in,
- false);
-}
-if (info-when WHEN_ACTIVE) {
-ret = testCompareXMLToXMLFiles(xml_in,
- info-different ? xml_out : xml_in,
- true);
-}
+if ((info-when WHEN_INACTIVE)
+testCompareXMLToXMLFiles(xml_in,
+ info-different ? xml_out : xml_in,
+ false) 0)
+goto cleanup;
+
+if ((info-when WHEN_ACTIVE)
+testCompareXMLToXMLFiles(xml_in,
+ info-different ? xml_out : xml_in,
+ true) 0)
+goto cleanup;
+
+ret = 0;
cleanup:
VIR_FREE(xml_in);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] Introduce new OOM testing support
From: Daniel P. Berrange berra...@redhat.com
The previous OOM testing support would re-run the entire main
method each iteration, failing a different malloc each time.
When a test suite has 'n' allocations, the number of repeats
requires is (n * (n + 1) ) / 2. This gets very large, very
quickly.
This new OOM testing support instead integrates at the
virtTestRun level, so each individual test case gets repeated,
instead of the entire test suite. This means the values of
'n' are orders of magnitude smaller.
The simple usage is
$ VIR_TEST_OOM=1 ./qemuxml2argvtest
...
29) QEMU XML-2-ARGV clock-utc ... OK
Test OOM for nalloc=36 OK
30) QEMU XML-2-ARGV clock-localtime ... OK
Test OOM for nalloc=36 OK
31) QEMU XML-2-ARGV clock-france ... OK
Test OOM for nalloc=38 .. OK
...
the second lines reports how many mallocs have to be failed, and thus
how many repeats of the test wil be run.
If it crashes, then running under valgrind will often show the problem
$ VIR_TEST_OOM=1 ../run valgrind ./qemuxml2argvtest
When debugging problems it is also helpful to select an individual
test case
$ VIR_TEST_RANGE=30 VIR_TEST_OOM=1 ../run valgrind ./qemuxml2argvtest
When things get really tricky, it is possible to request that just
specific allocs are failed. eg to fail allocs 5 - 12, use
$ VIR_TEST_RANGE=30 VIR_TEST_OOM=1:5-12 ../run valgrind ./qemuxml2argvtest
In the worse case, you might want to know the stack trace of the
alloc which was failed then VIR_TEST_OOM_TRACE can be set. If it
is set to 1 then it will only print if it things a mistake happened.
This is often not reliable, so setting it to 2 will make it print
the stack trace for every alloc that is failed.
$ VIR_TEST_OOM_TRACE=2 VIR_TEST_RANGE=30 VIR_TEST_OOM=1:5-5 ../run valgrind
./qemuxml2argvtest
30) QEMU XML-2-ARGV clock-localtime ... OK
Test OOM for nalloc=36 !virAllocN
/home/berrange/src/virt/libvirt/src/util/viralloc.c:180
virHashCreateFull
/home/berrange/src/virt/libvirt/src/util/virhash.c:144
virDomainDefParseXML
/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11745
virDomainDefParseNode
/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12646
virDomainDefParse
/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12590
testCompareXMLToArgvFiles
/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:106
virtTestRun
/home/berrange/src/virt/libvirt/tests/testutils.c:250
mymain
/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:418 (discriminator 2)
virtTestMain
/home/berrange/src/virt/libvirt/tests/testutils.c:750
??
??:0
_start
??:?
FAILED
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tests/cputest.c | 3 +-
tests/qemuargv2xmltest.c | 14 ++--
tests/qemuxml2argvtest.c | 12 ++-
tests/qemuxmlnstest.c| 18 +++--
tests/testutils.c| 189 ++-
tests/testutils.h| 2 +
6 files changed, 216 insertions(+), 22 deletions(-)
diff --git a/tests/cputest.c b/tests/cputest.c
index 8e3640b..17cb6af 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -476,7 +476,8 @@ cpuTestRun(const char *name, const struct data *data)
if (virTestGetDebug()) {
char *log;
if ((log = virtTestLogContentAndReset())
- strlen(log) 0)
+log != NULL
+strlen(log) 0)
fprintf(stderr, \n%s\n, log);
VIR_FREE(log);
}
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 6dd8bb0..92fc89a 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -42,7 +42,6 @@ static int testCompareXMLToArgvFiles(const char *xml,
char *cmd = NULL;
int ret = -1;
virDomainDefPtr vmdef = NULL;
-char *log;
if (virtTestLoadFile(cmdfile, cmd) 0)
goto fail;
@@ -53,13 +52,16 @@ static int testCompareXMLToArgvFiles(const char *xml,
cmd, NULL, NULL, NULL)))
goto fail;
-if ((log = virtTestLogContentAndReset()) == NULL)
-goto fail;
-if ((*log != '\0') != expect_warning) {
+if (!virtTestOOMActive()) {
+char *log;
+if ((log = virtTestLogContentAndReset()) == NULL)
+goto fail;
+if ((*log != '\0') != expect_warning) {
+VIR_FREE(log);
+goto fail;
+}
VIR_FREE(log);
-goto fail;
}
-VIR_FREE(log);
if (!(actualxml = virDomainDefFormat(vmdef, 0)))
goto fail;
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 5658792..4feae7c 100644
--- a/tests/qemuxml2argvtest.c
+++
[libvirt] [PATCH] spec: Clean up distribution of ChangeLog (and others)
- Move COPYING* to libvirt-client, so every package pulls them in
- Move AUTHORS ChangeLog.gz NEWS README TODO from -daemon to -docs
- Drop duplicate distribution of docs in -python
---
libvirt.spec.in | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f899149..d294aad 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1723,6 +1723,8 @@ fi
%files docs
%defattr(-, root, root)
+%doc AUTHORS ChangeLog.gz NEWS README TODO
+
# Website
%dir %{_datadir}/doc/libvirt-docs-%{version}
%dir %{_datadir}/doc/libvirt-docs-%{version}/html
@@ -1739,7 +1741,6 @@ fi
%files daemon
%defattr(-, root, root)
-%doc AUTHORS ChangeLog.gz NEWS README COPYING COPYING.LESSER TODO
%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/
%if %{with_network}
@@ -2011,7 +2012,7 @@ fi
%files client -f %{name}.lang
%defattr(-, root, root)
-%doc AUTHORS ChangeLog.gz NEWS README COPYING COPYING.LESSER TODO
+%doc COPYING COPYING.LESSER
%config(noreplace) %{_sysconfdir}/libvirt/libvirt.conf
%if %{with_lxc}
@@ -2101,7 +2102,6 @@ fi
%files python
%defattr(-, root, root)
-%doc AUTHORS NEWS README COPYING COPYING.LESSER
%{_libdir}/python*/site-packages/libvirt.py*
%{_libdir}/python*/site-packages/libvirt_qemu.py*
%{_libdir}/python*/site-packages/libvirt_lxc.py*
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/9] test: Allow specifying object runstate in driver XML
On 08/30/2013 12:43 PM, Eric Blake wrote:
On 08/30/2013 10:03 AM, Cole Robinson wrote:
When passing in custom driver XML, allow a block like
domain xmlns:test='http://libvirt.org/schemas/domain/test/1.0'
...
test:runstate5/test:runstate
Since the enum virDomainState is part of our public API, I'm not worried
about the numbers ever being tied to the wrong state. But wouldn't it
be nicer for the XML to use a string conversion of a name, rather than
just a raw number? On the other hand, this is just for testing
purposes, so I can live with it.
Yeah I think for something like this the only people who will use it are
capable of getting the domain state numbers, and it keeps the code simpler.
/domain
This is only read at initial driver start time, and sets the initial
run state of the object. This is handy for UI testing.
It's only wired up for domains, since that's the only conf/
infrastructure that supports namespaces at the moment.
---
src/test/test_driver.c | 91
++
1 file changed, 84 insertions(+), 7 deletions(-)
+
+tmp = virXPathUInt(string(./test:runstate), ctxt, tmpuint);
+if (tmp == 0) {
+if (tmpuint = VIR_DOMAIN_LAST) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(runstate '%d' out of range'), tmpuint);
+goto error;
+}
+nsdata-runstate = tmpuint;
Should we also reject VIR_DOMAIN_NOSTATE?
But isn't it a valid state? I know xen used to return it quite a bit but that
was ages ago. But if any libvirt version returned it then it's useful for apps
to test.
+};
+
+/* All our XML extensions are write only, so we only need to parse */
Maybe s/write only/input only/
ACK, but not until after the release.
Thanks, pushed now with that change.
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Clean up distribution of ChangeLog (and others)
On 09/25/2013 11:23 AM, Cole Robinson wrote: - Move COPYING* to libvirt-client, so every package pulls them in - Move AUTHORS ChangeLog.gz NEWS README TODO from -daemon to -docs - Drop duplicate distribution of docs in -python --- libvirt.spec.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) ACK. Wasn't there a Fedora BZ about this? If so, can you mention it in the commit message? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
