Re: [libvirt] [PATCHv2 2/5] Add functions to track virtio-serial addresses
On Mon, Mar 23, 2015 at 05:46:19PM -0400, John Ferlan wrote:
On 03/17/2015 07:41 AM, Ján Tomko wrote:
Create a sorted array of virtio-serial controllers.
Each of the elements contains the controller index
and a bitmap of available ports.
Buses are not tracked, because they aren't supported by QEMU.
---
src/conf/domain_addr.c | 348
+++
src/conf/domain_addr.h | 56
src/libvirt_private.syms | 9 ++
3 files changed, 413 insertions(+)
I assumed the ACK to 1/5 sticks...
+
+static void
+virDomainVirtioSerialControllerFree(virDomainVirtioSerialControllerPtr
cont)
Should the Free routine take a pointer so that when we VIR_FREE the
pointer the caller doesn't have to worry about setting their copy to NULL?
None of the callers worry about that for this function. For the other
function, I like sticking to the existing convention:
*Free routines usually take a copy of the address, just like
virBitmapFree below.
I think
virFuncFree(foo-ptr);
looks more tidy than
virFuncFree((foo-ptr));
And in most of the cases, foo gets freed anyway.
+{
+if (cont) {
+virBitmapFree(cont-ports);
+VIR_FREE(cont);
+}
+}
+
+static ssize_t
+virDomainVirtioSerialAddrPlaceController(virDomainVirtioSerialAddrSetPtr
addrs,
+
virDomainVirtioSerialControllerPtr cont)
+{
+size_t i;
+
+for (i = 0; i addrs-ncontrollers; i++) {
+if (addrs-controllers[i]-idx = cont-idx)
+return i;
+}
Would entries controller type='virtio-serial' index='1' ports='4'
and controller type='virtio-serial' index='1' be rejected elsewhere?
I would think index would be unique but this algorithm seems to be
fine and happy with it.
For user-specified controllers, duplicate indexes are rejected by
virDomainDefRejectDuplicateControllers, so adding a controller
with a non-unique index would be a bug in the auto-allocation logic.
I'll squash this in:
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index d9d01fc..cda9ad2 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -754,7 +754,14 @@
virDomainVirtioSerialAddrPlaceController(virDomainVirtioSerialAddrSetPtr addrs,
size_t i;
for (i = 0; i addrs-ncontrollers; i++) {
-if (addrs-controllers[i]-idx = cont-idx)
+if (addrs-controllers[i]-idx == cont-idx) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(virtio serial controller with index %u already
exists
+ in the address set),
+ cont-idx);
+return -2;
+}
+if (addrs-controllers[i]-idx cont-idx)
return i;
}
return -1;
@@ -804,7 +811,8 @@
virDomainVirtioSerialAddrSetAddController(virDomainVirtioSerialAddrSetPtr addrs,
goto cleanup;
cnt-idx = cont-idx;
-insertAt = virDomainVirtioSerialAddrPlaceController(addrs, cnt);
+if ((insertAt = virDomainVirtioSerialAddrPlaceController(addrs, cnt)) -1)
+goto cleanup;
if (VIR_INSERT_ELEMENT(addrs-controllers, insertAt,
addrs-ncontrollers, cnt) 0)
goto cleanup;
+/* virDomainVirtioSerialAddrSetRemoveController
+ *
+ * Removes a virtio serial controller from the address set.
+ */
+int
+virDomainVirtioSerialAddrSetRemoveController(virDomainVirtioSerialAddrSetPtr
addrs,
+ virDomainControllerDefPtr
cont)
+{
+int ret = -1;
+ssize_t pos;
+
+if (cont-type != VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
+return 0;
+
+VIR_DEBUG(Removing virtio serial controller index %u
+ from the address set, cont-idx);
+
+pos = virDomainVirtioSerialAddrFindController(addrs, cont-idx);
+
+if (pos = 0
+VIR_DELETE_ELEMENT(addrs-controllers, pos, addrs-ncontrollers)
0)
+goto cleanup;
+
If 'pos' 0, we return 0 (and perhaps leak something). OTOH, the
controller was never added and the caller never checks status, maybe
this should just be void (wonder why Coverity didn't whine)...
There's nothing to be leaked. Coverity only whines if some callers check
the return value and some don't.
I'll change the return type to void.
+
+/* virDomainVirtioSerialAddrRelease
+ *
+ * Release the virtio serial address of the device
+ */
+int
+virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs,
+ virDomainDeviceInfoPtr info)
+{
+virBitmapPtr map;
+char *str = NULL;
+int ret = -1;
+ssize_t i;
+
+if (info-type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL ||
+info-addr.vioserial.port == 0)
+return 0;
+
+VIR_DEBUG(Releasing virtio serial %u %u,
Re: [libvirt] [PATCH 4/6] qemu: check if domain is really active when do setvcpus with --live
On 03/24/2015 05:31 PM, Pavel Hrdina wrote:
On Tue, Mar 24, 2015 at 02:27:42PM +0800, lhuang wrote:
On 03/20/2015 10:39 PM, Pavel Hrdina wrote:
From: Luyao Huang lhu...@redhat.com
Commit e105dc9 fix setting vcpus for offline domain, but forget check
if vm is active when pass VIR_DOMAIN_AFFECT_LIVE flags.
# virsh setvcpus test3 4 --live
error: Failed to create controller cpu for group: No such file or directory
Add a check if we pass VIR_DOMAIN_AFFECT_LIVE flags.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204006
Signed-off-by: Luyao Huang lhu...@redhat.com
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_driver.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4942712..c4d96bd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4881,6 +4881,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup;
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(domain is not running));
+goto endjob;
+}
+}
+
if (flags VIR_DOMAIN_AFFECT_LIVE !(flags VIR_DOMAIN_VCPU_GUEST)) {
if (virCgroupNewEmulator(priv-cgroup, false, cgroup_temp) 0)
goto endjob;
@@ -4947,12 +4955,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
if (ncpuinfo 0)
goto endjob;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain is not running));
-goto endjob;
-}
-
if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) 0)
goto endjob;
Sorry, i forgot there is a function virDomainLiveConfigHelperMethod(),
move this function just after qemuDomainObjBeginJob() maybe a good way
to fix this issue. Also virDomainLiveConfigHelperMethod() may change
flags, so it should be done more early (as soon as possible after set a
lock to vm).
I've already sent a patch 7/6 to move that function. I realized that right
after I've sent this series to mailing list.
Pavel
Okay, I think i missed patch 7/6 when i looked these patches :)
Thanks for your reply
Luyao
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib] build-sys: Fix libtoolize detection in autogen.sh
autogen.sh is currently checking for the libtool binary, but
it's libtoolize which is needed by autoreconf, not libtool.
These binaries are packaged separately on Debian/Ubuntu so this can
cause actual issues on some systems.
Bug reported by Frederic Peters
---
autogen.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/autogen.sh b/autogen.sh
index 8030ab1..4f7135f 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -10,7 +10,7 @@ cd $srcdir
DIE=0
-for prog in intltoolize autoreconf automake autoconf libtool
+for prog in intltoolize autoreconf automake autoconf libtoolize
do
($prog --version) /dev/null /dev/null 21 || {
echo
--
2.3.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] conf: Add support for storage state directory
On 03/24/2015 06:06 AM, Erik Skultety wrote:
Before introducing necessary changes to storage_driver.c, first prepare
our structures for storage state XML support.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/conf/storage_conf.h | 1 +
src/storage/storage_driver.c | 1 +
2 files changed, 2 insertions(+)
I had started down this path before for a different bug/issue:
http://www.redhat.com/archives/libvir-list/2015-January/msg00465.html
but abandoned the stateDir because it was felt it wasn't necessary. I
recall running into a few interesting issues with stateDir including
fixing one issue seen during testing that didn't hit the list. Good
news is I still have the patches in a branch somewhere if you're
interested. 1 2 are on list... The 3rd one in the archive was a
solution to the particular problem - that was rejected and a different
solution was pushed.
In any case, I do suggest looking at 1 2, plus I can send you an
adjustment to 1 to resolve some condition I ran into, but cannot recall
the details.
John
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 4584075..8ccc947 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -293,6 +293,7 @@ struct _virStorageDriverState {
char *configDir;
char *autostartDir;
+char *stateDir;
bool privileged;
};
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 64ea770..e088ffa 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -261,6 +261,7 @@ storageStateCleanup(void)
VIR_FREE(driver-configDir);
VIR_FREE(driver-autostartDir);
+VIR_FREE(driver-stateDir);
storageDriverUnlock();
virMutexDestroy(driver-lock);
VIR_FREE(driver);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] RFC: Add domain vmport attribute
Hi On Mon, Mar 23, 2015 at 11:42 PM, Daniel P. Berrange berra...@redhat.com wrote: I think we'd normally place this kind of attribute in the features/features block rather than here. eg see the toggle for turning on/off hyper-v emulation. Are you suggesting this: vmport state=on/off/? and if it's not there it would use the default behaviour. -- Marc-André Lureau -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib] build-sys: Fix libtoolize detection in autogen.sh
On 24.03.2015 13:51, Christophe Fergeau wrote:
autogen.sh is currently checking for the libtool binary, but
it's libtoolize which is needed by autoreconf, not libtool.
These binaries are packaged separately on Debian/Ubuntu so this can
cause actual issues on some systems.
Bug reported by Frederic Peters
---
autogen.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/autogen.sh b/autogen.sh
index 8030ab1..4f7135f 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -10,7 +10,7 @@ cd $srcdir
DIE=0
-for prog in intltoolize autoreconf automake autoconf libtool
+for prog in intltoolize autoreconf automake autoconf libtoolize
do
($prog --version) /dev/null /dev/null 21 || {
echo
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] tests: xml2xml: Refactor the qemu xml 2 xml test
To allow adding more tests, refactor the XML-2-XML test so that the
files are not reloaded always and clarify the control flow.
Result of this changes is that the active and inactive portions of the
XML are tested in separate steps rather than one test step.
---
tests/qemuxml2xmltest.c | 214 +++-
1 file changed, 140 insertions(+), 74 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0f16d5e..627edca 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -22,11 +22,30 @@
static virQEMUDriver driver;
+enum {
+WHEN_INACTIVE = 1,
+WHEN_ACTIVE = 2,
+WHEN_EITHER = 3,
+};
+
+struct testInfo {
+char *inName;
+char *inFile;
+
+char *outActiveName;
+char *outActiveFile;
+
+char *outInactiveName;
+char *outInactiveFile;
+};
+
static int
-testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live)
+testXML2XMLHelper(const char *inxml,
+ const char *inXmlData,
+ const char *outxml,
+ const char *outXmlData,
+ bool live)
{
-char *inXmlData = NULL;
-char *outXmlData = NULL;
char *actual = NULL;
int ret = -1;
virDomainDefPtr def = NULL;
@@ -35,11 +54,6 @@ testCompareXMLToXMLFiles(const char *inxml, const char
*outxml, bool live)
if (!live)
format_flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
-if (virtTestLoadFile(inxml, inXmlData) 0)
-goto fail;
-if (virtTestLoadFile(outxml, outXmlData) 0)
-goto fail;
-
if (!(def = virDomainDefParseString(inXmlData, driver.caps, driver.xmlopt,
QEMU_EXPECTED_VIRT_TYPES,
parse_flags)))
goto fail;
@@ -58,82 +72,120 @@ testCompareXMLToXMLFiles(const char *inxml, const char
*outxml, bool live)
}
ret = 0;
+
fail:
-VIR_FREE(inXmlData);
-VIR_FREE(outXmlData);
VIR_FREE(actual);
virDomainDefFree(def);
return ret;
}
-enum {
-WHEN_INACTIVE = 1,
-WHEN_ACTIVE = 2,
-WHEN_EITHER = 3,
-};
-struct testInfo {
-const char *name;
-bool different;
-int when;
-};
+static int
+testXML2XMLActive(const void *opaque)
+{
+const struct testInfo *info = opaque;
+
+return testXML2XMLHelper(info-inName,
+ info-inFile,
+ info-outActiveName,
+ info-outActiveFile,
+ true);
+}
+
static int
-testCompareXMLToXMLHelper(const void *data)
+testXML2XMLInactive(const void *opaque)
{
-const struct testInfo *info = data;
-char *xml_in = NULL;
-char *xml_out = NULL;
-char *xml_out_active = NULL;
-char *xml_out_inactive = NULL;
-int ret = -1;
+const struct testInfo *info = opaque;
+
+return testXML2XMLHelper(info-inName,
+ info-inFile,
+ info-outInactiveName,
+ info-outInactiveFile,
+ false);
+}
+
-if (virAsprintf(xml_in, %s/qemuxml2argvdata/qemuxml2argv-%s.xml,
-abs_srcdir, info-name) 0 ||
-virAsprintf(xml_out, %s/qemuxml2xmloutdata/qemuxml2xmlout-%s.xml,
-abs_srcdir, info-name) 0 ||
-virAsprintf(xml_out_active,
-%s/qemuxml2xmloutdata/qemuxml2xmlout-%s-active.xml,
-abs_srcdir, info-name) 0 ||
-virAsprintf(xml_out_inactive,
-%s/qemuxml2xmloutdata/qemuxml2xmlout-%s-inactive.xml,
-abs_srcdir, info-name) 0)
-goto cleanup;
-
-if ((info-when WHEN_INACTIVE)) {
-char *out;
-if (!info-different)
-out = xml_in;
-else if (virFileExists(xml_out_inactive))
-out = xml_out_inactive;
-else
-out = xml_out;
-
-if (testCompareXMLToXMLFiles(xml_in, out, false) 0)
-goto cleanup;
+static void
+testInfoFree(struct testInfo *info)
+{
+VIR_FREE(info-inName);
+VIR_FREE(info-inFile);
+
+VIR_FREE(info-outActiveName);
+VIR_FREE(info-outActiveFile);
+
+VIR_FREE(info-outInactiveName);
+VIR_FREE(info-outInactiveFile);
+}
+
+
+static int
+testInfoSet(struct testInfo *info,
+const char *name,
+bool different,
+int when)
+{
+if (virAsprintf(info-inName, %s/qemuxml2argvdata/qemuxml2argv-%s.xml,
+abs_srcdir, name) 0)
+goto error;
+
+if (virtTestLoadFile(info-inName, info-inFile) 0)
+goto error;
+
+if (when WHEN_INACTIVE) {
+if (different) {
+if (virAsprintf(info-outInactiveName,
+
%s/qemuxml2xmloutdata/qemuxml2xmlout-%s-inactive.xml,
+ abs_srcdir, name) 0)
+goto error;
+
+if (!virFileExists(info-outInactiveName)) {
+
[libvirt] [PATCH 2/4] util: buffer: Add support for adding text blocks with indentation
The current auto-indentation buffer code applies indentation only on
complete strings. To allow adding a string containing newlines and
having it properly indented this patch adds virBufferAddStr.
---
src/libvirt_private.syms | 1 +
src/util/virbuffer.c | 38
src/util/virbuffer.h | 1 +
tests/virbuftest.c | 50
4 files changed, 90 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 33222f0..0beb44f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1099,6 +1099,7 @@ virBitmapToData;
virBufferAdd;
virBufferAddBuffer;
virBufferAddChar;
+virBufferAddStr;
virBufferAdjustIndent;
virBufferAsprintf;
virBufferCheckErrorInternal;
diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 0089d1b..50d953e 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -756,3 +756,41 @@ virBufferTrim(virBufferPtr buf, const char *str, int len)
buf-use -= len 0 ? len2 : len;
buf-content[buf-use] = '\0';
}
+
+
+/**
+ * virBufferAddStr:
+ * @buf: the buffer to append to
+ * @str: string to append
+ *
+ * Appends @str to @buffer. Applies autoindentation on the separate lines of
+ * @str.
+ */
+void
+virBufferAddStr(virBufferPtr buf,
+const char *str)
+{
+size_t len = 0;
+const char *start = str;
+
+if (!buf || !str || buf-error)
+return;
+
+while (*str) {
+len++;
+
+if (*str == '\n') {
+virBufferAdd(buf, start, len);
+str++;
+len = 0;
+start = str;
+
+continue;
+}
+
+str++;
+}
+
+if (len 0)
+virBufferAdd(buf, start, len);
+}
diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h
index 24e81c7..144a1ba 100644
--- a/src/util/virbuffer.h
+++ b/src/util/virbuffer.h
@@ -96,5 +96,6 @@ void virBufferAdjustIndent(virBufferPtr buf, int indent);
int virBufferGetIndent(const virBuffer *buf, bool dynamic);
void virBufferTrim(virBufferPtr buf, const char *trim, int len);
+void virBufferAddStr(virBufferPtr buf, const char *str);
#endif /* __VIR_BUFFER_H__ */
diff --git a/tests/virbuftest.c b/tests/virbuftest.c
index f964feb..067a77e 100644
--- a/tests/virbuftest.c
+++ b/tests/virbuftest.c
@@ -310,6 +310,44 @@ static int testBufAddBuffer(const void *data
ATTRIBUTE_UNUSED)
return ret;
}
+struct testBufAddStrData {
+const char *data;
+const char *expect;
+};
+
+static int
+testBufAddStr(const void *opaque ATTRIBUTE_UNUSED)
+{
+const struct testBufAddStrData *data = opaque;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char *actual;
+int ret = -1;
+
+virBufferAddLit(buf, c\n);
+virBufferAdjustIndent(buf, 2);
+virBufferAddStr(buf, data-data);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, /c);
+
+if (!(actual = virBufferContentAndReset(buf))) {
+TEST_ERROR(buf is empty);
+goto cleanup;
+}
+
+if (STRNEQ_NULLABLE(actual, data-expect)) {
+TEST_ERROR(testBufAddStr(): Strings don't match:\n
+ Expected:\n%s\nActual:\n%s\n,
+ data-expect, actual);
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(actual);
+return ret;
+}
+
static int
mymain(void)
@@ -330,6 +368,18 @@ mymain(void)
DO_TEST(Trim, testBufTrim, 0);
DO_TEST(AddBuffer, testBufAddBuffer, 0);
+#define DO_TEST_ADD_STR(DATA, EXPECT) \
+do { \
+struct testBufAddStrData info = { DATA, EXPECT }; \
+if (virtTestRun(Buf: AddStr, testBufAddStr, info) 0) \
+ret = -1; \
+} while (0)
+
+DO_TEST_ADD_STR(, c\n/c);
+DO_TEST_ADD_STR(a/, c\n a//c);
+DO_TEST_ADD_STR(a/\n, c\n a/\n/c);
+DO_TEST_ADD_STR(b\n a/\n/b\n, c\n b\na/\n
/b\n/c);
+
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] test: qemuxml2xml: Test status XML formatting and parsing
Recently we've fixed a bug where the status XML could not be parsed as
the parser used absolute path XPath queries. This test enhancement tests
all XML files used in the qemu-xml-2-xml test as a part of a status XML
snippet to see whether they are parsed correctly. The status XML-2-XML is
currently tested in 223 cases with this patch.
---
src/conf/domain_conf.c | 4 +-
src/conf/domain_conf.h | 9
src/libvirt_private.syms | 2 +
tests/qemuxml2xmltest.c | 107 +++
4 files changed, 120 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d633f93..d28b62a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15580,7 +15580,7 @@ virDomainDefParseNode(xmlDocPtr xml,
}
-static virDomainObjPtr
+virDomainObjPtr
virDomainObjParseNode(xmlDocPtr xml,
xmlNodePtr root,
virCapsPtr caps,
@@ -21252,7 +21252,7 @@ virDomainDefFormat(virDomainDefPtr def, unsigned int
flags)
}
-static char *
+char *
virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
virDomainObjPtr obj,
unsigned int flags)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bceb2d7..608f61f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2565,6 +2565,12 @@ virDomainDefPtr virDomainDefParseNode(xmlDocPtr doc,
virDomainXMLOptionPtr xmlopt,
unsigned int expectedVirtTypes,
unsigned int flags);
+virDomainObjPtr virDomainObjParseNode(xmlDocPtr xml,
+ xmlNodePtr root,
+ virCapsPtr caps,
+ virDomainXMLOptionPtr xmlopt,
+ unsigned int expectedVirtTypes,
+ unsigned int flags);
virDomainObjPtr virDomainObjParseFile(const char *filename,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
@@ -2580,6 +2586,9 @@ unsigned int virDomainDefFormatConvertXMLFlags(unsigned
int flags);
char *virDomainDefFormat(virDomainDefPtr def,
unsigned int flags);
+char *virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
+ virDomainObjPtr obj,
+ unsigned int flags);
int virDomainDefFormatInternal(virDomainDefPtr def,
unsigned int flags,
virBufferPtr buf);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0beb44f..4ab8638 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -364,6 +364,7 @@ virDomainNostateReasonTypeFromString;
virDomainNostateReasonTypeToString;
virDomainObjAssignDef;
virDomainObjCopyPersistentDef;
+virDomainObjFormat;
virDomainObjGetMetadata;
virDomainObjGetPersistentDef;
virDomainObjGetState;
@@ -382,6 +383,7 @@ virDomainObjListNumOfDomains;
virDomainObjListRemove;
virDomainObjListRemoveLocked;
virDomainObjNew;
+virDomainObjParseNode;
virDomainObjSetDefTransient;
virDomainObjSetMetadata;
virDomainObjSetState;
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 627edca..b419231 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -106,6 +106,109 @@ testXML2XMLInactive(const void *opaque)
}
+static const char testStatusXMLPrefix[] =
+domstatus state='running' reason='booted' pid='3803518'\n
+ taint flag='high-privileges'/\n
+ monitor path='/var/lib/libvirt/qemu/test.monitor' json='1' type='unix'/\n
+ vcpus\n
+vcpu pid='3803519'/\n
+ /vcpus\n
+ qemuCaps\n
+flag name='vnc-colon'/\n
+flag name='no-reboot'/\n
+flag name='drive'/\n
+flag name='name'/\n
+flag name='uuid'/\n
+flag name='vnet-hdr'/\n
+flag name='qxl.vgamem_mb'/\n
+flag name='qxl-vga.vgamem_mb'/\n
+flag name='pc-dimm'/\n
+ /qemuCaps\n
+ devices\n
+device alias='balloon0'/\n
+device alias='video0'/\n
+device alias='serial0'/\n
+device alias='net0'/\n
+device alias='usb'/\n
+ /devices\n;
+
+static const char testStatusXMLSuffix[] =
+/domstatus\n;
+
+
+static int
+testCompareStatusXMLToXMLFiles(const void *opaque)
+{
+const struct testInfo *data = opaque;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+xmlDocPtr xml = NULL;
+virDomainObjPtr obj = NULL;
+char *expect = NULL;
+char *actual = NULL;
+char *source = NULL;
+int ret = -1;
+int keepBlanksDefault = xmlKeepBlanksDefault(0);
+
+/* construct faked source status XML */
+virBufferAdd(buf, testStatusXMLPrefix, -1);
+virBufferAdjustIndent(buf, 2);
+virBufferAddStr(buf, data-inFile);
+virBufferAdjustIndent(buf, -2);
+virBufferAdd(buf, testStatusXMLSuffix, -1);
+
+if (!(source = virBufferContentAndReset(buf)))
Re: [libvirt] [libvirt-glib] build-sys: Fix libtoolize detection in autogen.sh
On Tue, Mar 24, 2015 at 02:22:13PM +0100, Michal Privoznik wrote:
On 24.03.2015 13:51, Christophe Fergeau wrote:
autogen.sh is currently checking for the libtool binary, but
it's libtoolize which is needed by autoreconf, not libtool.
These binaries are packaged separately on Debian/Ubuntu so this can
cause actual issues on some systems.
Bug reported by Frederic Peters
---
autogen.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/autogen.sh b/autogen.sh
index 8030ab1..4f7135f 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -10,7 +10,7 @@ cd $srcdir
DIE=0
-for prog in intltoolize autoreconf automake autoconf libtool
+for prog in intltoolize autoreconf automake autoconf libtoolize
do
($prog --version) /dev/null /dev/null 21 || {
echo
ACK
Thanks, pushed.
Christophe
pgpSX9dzcXQUw.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Give hint about -noTSX CPU model
Because of the microcode update to Haswell/Broadwell CPUs, existing
domains using these CPUs may fail to start even though they used to run
just fine. To help users solve this issue we try to suggest switching to
-noTSX variant of the CPU model:
virsh # start cd
error: Failed to start domain cd
error: unsupported configuration: guest and host CPU are not
compatible: Host CPU does not provide required features: rtm, hle;
try using 'Haswell-noTSX' CPU model
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
This patch depends on the previous patch which introduces the new
*-noTSX models.
src/qemu/qemu_command.c | 38 +-
1 file changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7e6b95c..cabb8b2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6807,6 +6807,7 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
size_t ncpus = 0;
char **cpus = NULL;
virCPUDataPtr data = NULL;
+virCPUDataPtr hostData = NULL;
char *compare_msg = NULL;
virCPUCompareResult cmp;
const char *preferred;
@@ -6838,16 +6839,42 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
/* For non-KVM, CPU features are emulated, so host compat doesn't matter */
if (compareAgainstHost) {
+bool noTSX = false;
+
cmp = cpuGuestData(host, cpu, data, compare_msg);
switch (cmp) {
case VIR_CPU_COMPARE_INCOMPATIBLE:
+if (cpuEncode(host-arch, host, NULL, hostData,
+ NULL, NULL, NULL, NULL) == 0
+(!cpuHasFeature(hostData, hle) ||
+ !cpuHasFeature(hostData, rtm))
+(STREQ_NULLABLE(cpu-model, Haswell) ||
+ STREQ_NULLABLE(cpu-model, Broadwell)))
+noTSX = true;
+
if (compare_msg) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(guest and host CPU are not compatible: %s),
- compare_msg);
+if (noTSX) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(guest and host CPU are not compatible:
+ %s; try using '%s-noTSX' CPU model),
+ compare_msg, cpu-model);
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(guest and host CPU are not compatible:
+ %s),
+ compare_msg);
+}
} else {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(guest CPU is not compatible with host CPU));
+if (noTSX) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(guest CPU is not compatible with host
+ CPU; try using '%s-noTSX' CPU model),
+ cpu-model);
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(guest CPU is not compatible with host
+ CPU));
+}
}
/* fall through */
case VIR_CPU_COMPARE_ERROR:
@@ -6941,6 +6968,7 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
virObjectUnref(caps);
VIR_FREE(compare_msg);
cpuDataFree(data);
+cpuDataFree(hostData);
virCPUDefFree(guest);
virCPUDefFree(cpu);
return ret;
--
2.3.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Test status XML formatting and parsing
A recent bug showed that the status XML parsing is not tested. Add test cases based on existing tests in the XML-2-XML test to excercise the parser. Additionally this series fixes also a memleak of the domain device alias list. Peter Krempa (4): qemu: domain: Don't leak device alias list util: buffer: Add support for adding text blocks with indentation tests: xml2xml: Refactor the qemu xml 2 xml test test: qemuxml2xml: Test status XML formatting and parsing src/conf/domain_conf.c | 4 +- src/conf/domain_conf.h | 9 ++ src/libvirt_private.syms | 3 + src/qemu/qemu_domain.c | 1 + src/util/virbuffer.c | 38 ++ src/util/virbuffer.h | 1 + tests/qemuxml2xmltest.c | 309 --- tests/virbuftest.c | 50 8 files changed, 345 insertions(+), 70 deletions(-) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] qemu: domain: Don't leak device alias list
While adding tests for status XML parsing and formatting I've noticed that the device alias list is leaked. ==763001== 81 (48 direct, 33 indirect) bytes in 1 blocks are definitely lost in loss record 414 of 514 ==763001==at 0x4C2B8F0: calloc (vg_replace_malloc.c:623) ==763001==by 0x6ACF70F: virAllocN (viralloc.c:191) ==763001==by 0x447B64: qemuDomainObjPrivateXMLParse (qemu_domain.c:727) ==763001==by 0x6B848F9: virDomainObjParseXML (domain_conf.c:15491) ==763001==by 0x6B84CAC: virDomainObjParseNode (domain_conf.c:15608) --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 655afb9..1cf1aee 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -439,6 +439,7 @@ qemuDomainObjPrivateFree(void *data) VIR_FREE(priv-origname); virCondDestroy(priv-unplugFinished); +virStringFreeList(priv-qemuDevices); virChrdevFree(priv-devs); /* This should never be non-NULL if we get here, but just in case... */ -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] libvirt/libxl implemetation of get_online_cpu / virNodeGetCPUMap?
On Tuesday 24 February 2015 08:30 PM, Anthony PERARD wrote: On Tue, Feb 24, 2015 at 01:22:19PM +, Daniel P. Berrange wrote: On Tue, Feb 24, 2015 at 01:15:57PM +, Anthony PERARD wrote: On Tue, Feb 24, 2015 at 12:46:44PM +, Daniel P. Berrange wrote: On Tue, Feb 24, 2015 at 12:41:01PM +, Anthony PERARD wrote: Hi, A recent OpenStack nova commit make use of virNodeGetCPUMap to get the list of online cpu of a host. But this API is not implemented for the libvirt xen driver. The commit: Add handling for offlined CPUs to the nova libvirt driver. https://review.openstack.org/gitweb?p=openstack/nova.git;a=commitdiff;h=0696a5cd5f0fdc08951a074961bb8ce0c3310086 FWIW, this should not impact Xen based on my understanding. The code path in question should only be used when Nova is setup todo NUMA pinning support, and that is not supported with Xen in OpenStack, only KVM. Did it actually cause failures for you, or are you simply keeping track of all used APIs in Nova as a sanity check ? It prevent nova from starting. I do the setup with DevStack. The error: libvirtError: this function is not supported by the connection driver: virNodeGetCPUMap And a part of the traceback: File /opt/stack/nova/nova/openstack/common/service.py, line 491, in run_service service.start() File /opt/stack/nova/nova/service.py, line 181, in start self.manager.pre_start_hook() File /opt/stack/nova/nova/compute/manager.py, line 1188, in pre_start_hook self.update_available_resource(nova.context.get_admin_context()) File /opt/stack/nova/nova/compute/manager.py, line 6062, in update_available_resource rt.update_available_resource(context) File /opt/stack/nova/nova/compute/resource_tracker.py, line 315, in update_available_resource resources = self.driver.get_available_resource(self.nodename) File /opt/stack/nova/nova/virt/libvirt/driver.py, line 4896, in get_available_resource numa_topology = self._get_host_numa_topology() File /opt/stack/nova/nova/virt/libvirt/driver.py, line 4749, in _get_host_numa_topology online_cpus = self._host.get_online_cpus() File /opt/stack/nova/nova/virt/libvirt/host.py, line 599, in get_online_cpus (cpus, cpu_map, online) = self.get_connection().getCPUMap() I'll look into why nova is going through NUMA code paths then. Oh damn, yes, I understand why now. Please file a bug against Nova for this, as we must fix it as a high pripority. It was certainly not my intention to break Xen when I approved this change I applied the patch, not getting this python libvirtError, but libvirt deamon is throwing error. 2015-03-24 08:46:31.169+: 1030: error : virNodeGetCPUMap:1342 : this function is not supported by the connection driver: virNodeGetCPUMap Here is the bug report: https://bugs.launchpad.net/nova/+bug/1425115 Regards, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] cpu: Add {Haswell,Broadwell}-noTSX CPU models
QEMU 2.3 adds these new models to cover Haswell and Broadwell CPUs with
updated microcode. Luckily, they also reverted former the machine type
specific changes to existing models. And since these changes were never
released, we don't need to hack around them in libvirt.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/cpu/cpu_map.xml| 18 ++-
tests/cputest.c| 15 ++-
tests/cputestdata/x86-Haswell-noTSX-nofallback.xml | 4 +
tests/cputestdata/x86-Haswell-noTSX.xml| 4 +
tests/cputestdata/x86-Haswell.xml | 6 +
tests/cputestdata/x86-baseline-7-result.xml| 4 +
tests/cputestdata/x86-baseline-7.xml | 24
tests/cputestdata/x86-baseline-8-result.xml| 4 +
tests/cputestdata/x86-baseline-8.xml | 28 +
...aswell-noTSX+Haswell,haswell,Haswell-result.xml | 6 +
...ll-noTSX+Haswell-noTSX,Haswell-noTSX-result.xml | 4 +
...+Haswell-noTSX,haswell,Haswell-noTSX-result.xml | 6 +
tests/cputestdata/x86-host-Haswell-noTSX.xml | 6 +
.../qemuxml2argv-cpu-Haswell-noTSX.args| 4 +
.../qemuxml2argv-cpu-Haswell-noTSX.xml | 21
.../qemuxml2argvdata/qemuxml2argv-cpu-Haswell.args | 4 +
.../qemuxml2argvdata/qemuxml2argv-cpu-Haswell.xml | 21
.../qemuxml2argv-cpu-Haswell2.args | 4 +
.../qemuxml2argvdata/qemuxml2argv-cpu-Haswell2.xml | 23
.../qemuxml2argv-cpu-Haswell3.args | 4 +
.../qemuxml2argvdata/qemuxml2argv-cpu-Haswell3.xml | 23
tests/qemuxml2argvtest.c | 8 ++
tests/testutilsqemu.c | 124 +++--
tests/testutilsqemu.h | 6 +
24 files changed, 331 insertions(+), 40 deletions(-)
create mode 100644 tests/cputestdata/x86-Haswell-noTSX-nofallback.xml
create mode 100644 tests/cputestdata/x86-Haswell-noTSX.xml
create mode 100644 tests/cputestdata/x86-Haswell.xml
create mode 100644 tests/cputestdata/x86-baseline-7-result.xml
create mode 100644 tests/cputestdata/x86-baseline-7.xml
create mode 100644 tests/cputestdata/x86-baseline-8-result.xml
create mode 100644 tests/cputestdata/x86-baseline-8.xml
create mode 100644
tests/cputestdata/x86-host-Haswell-noTSX+Haswell,haswell,Haswell-result.xml
create mode 100644
tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX,Haswell-noTSX-result.xml
create mode 100644
tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX,haswell,Haswell-noTSX-result.xml
create mode 100644 tests/cputestdata/x86-host-Haswell-noTSX.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell-noTSX.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell-noTSX.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell2.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell2.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell3.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell3.xml
diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 413148f..2110c0b 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -500,30 +500,40 @@
feature name='rdtscp'/
/model
-model name='Haswell'
+model name='Haswell-noTSX'
model name='SandyBridge'/
feature name='fma'/
feature name='pcid'/
feature name='movbe'/
feature name='fsgsbase'/
feature name='bmi1'/
- feature name='hle'/
feature name='avx2'/
feature name='smep'/
feature name='bmi2'/
feature name='erms'/
feature name='invpcid'/
+/model
+
+model name='Haswell'
+ model name='Haswell-noTSX'/
+ feature name='hle'/
feature name='rtm'/
/model
-model name='Broadwell'
- model name='Haswell'/
+model name='Broadwell-noTSX'
+ model name='Haswell-noTSX'/
feature name='3dnowprefetch'/
feature name='rdseed'/
feature name='adx'/
feature name='smap'/
/model
+model name='Broadwell'
+ model name='Broadwell-noTSX'/
+ feature name='hle'/
+ feature name='rtm'/
+/model
+
!-- AMD CPUs --
model name='athlon'
model name='pentiumpro'/
diff --git a/tests/cputest.c b/tests/cputest.c
index 449b7d1..bf7a48f 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -122,8 +122,10 @@ cpuTestLoadMultiXML(const char *arch,
goto cleanup;
n = virXPathNodeSet(/cpuTest/cpu, ctxt, nodes);
-if (n = 0 || (VIR_ALLOC_N(cpus, n) 0))
+if (n = 0 || (VIR_ALLOC_N(cpus, n) 0)) {
+fprintf(stderr, \nNo /cpuTest/cpu elements found in %s\n, xml);
goto cleanup;
+}
for (i = 0; i n; i++) {
ctxt-node = nodes[i];
Re: [libvirt] [PATCH] conf: fix running vm numa settings disappear after restart libvirtd
On Mon, Mar 23, 2015 at 20:11:19 +0800, Luyao Huang wrote:
On 03/23/2015 06:10 PM, Peter Krempa wrote:
On Thu, Mar 19, 2015 at 18:13:04 +0800, Luyao Huang wrote:
5bba61f introduce a issue : when start a vm with numa settings and
restart libvirtd, numa settings will disappear. Because when parse the
vm states file, there is no node in /domain/cpu/numa this place.
Change to use ./cpu/numa instead of /domain/cpu/numa.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/numa_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Indeed the status XML wraps the domain element with another container
element.
Yes, i just didn't know how to describe it in a right way when i wrote
this :)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index b92cb44..c34ba5f 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -663,10 +663,10 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def,
int ret = -1;
/* check if NUMA definition is present */
-if (!virXPathNode(/domain/cpu/numa[1], ctxt))
+if (!virXPathNode(./cpu/numa[1], ctxt))
return 0;
-if ((n = virXPathNodeSet(/domain/cpu/numa[1]/cell, ctxt, nodes))
= 0) {
+if ((n = virXPathNodeSet(./cpu/numa[1]/cell, ctxt, nodes)) = 0) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(NUMA topology defined without NUMA cells));
goto cleanup;
--
ACK, I'll push your patch after I figure out how to add tests for
problems like this.
I've pushed this patch as I've written tests that don't work without it.
I'll submit the tests later today.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hostdev: fix two bugs in virHostdevReAttachPCIDevices
On 03/22/2015 09:59 AM, Huanle Han wrote:
Bug 1: The the next element in the pcidevs is skipped after we
virPCIDeviceListDel the previous element.
Bug 2: virHostdevNetConfigRestore is called for store the hostdevs
which may be used by other domain.
Signed-off-by: Huanle Han hanxue...@gmail.com mailto:hanxue...@gmail.com
---
src/util/virhostdev.c | 25 +
1 file changed, 21 insertions(+), 4 deletions(-)
Should this be two separate patches? IOW: Are you fixing two separate
issues or essentially the same issue? Makes it easier to cherry-pick and
choose what may need to be backported elsewhere...
If there's a bug associated that would have been good to list as well
as perhaps an example or two... That is - what XML and command(s)
sequence(s) prompted you to write the patch(es)...
Also, my attempt to git am -3 your patch fails - perhaps it's your
mailer configuration. Did you use git send-email? or something else...
I'm seeing html format... So I'm only reviewing based on what I read.
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 9678e2b..ecbe5d8 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -785,7 +785,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr
hostdev_mgr,
* them and reset all the devices before re-attach.
* Attach mac and port profile parameters to devices
*/
^^^
I think the comment here could be updated to make it clear what's being
done. To just say Again 4 loops;... is a bit misleading since the only
loops I found in the code were in virHostdevPreparePCIDevices where
there are (currently) 9 such loops!
-for (i = 0; i virPCIDeviceListCount(pcidevs); i++) {
+for (i = 0; i virPCIDeviceListCount(pcidevs);) {
virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
virPCIDevicePtr activeDev = NULL;
@@ -806,6 +806,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr
hostdev_mgr,
}
virPCIDeviceListDel(hostdev_mgr-activePCIHostdevs, dev);
+i++;
}
Not sure I'm seeing why your fix works... Are you claiming that the
remove from other domain check (e.g. where the code continue;'s)
shouldn't be updating the 'i' value? All it seems you did was move the
i++ from the for line to after the second virPCIDeviceListDel in the
block... Which yes, does reduce the count of devices and I would think
should be the case that doesn't increment i...
If we have 4 devices [0, 1, 2, 3] and we remove [1], then there are 3
devices left and i would be 2, but the new array would be [0, 2, 3] and
thus we don't check [2].
Perhaps this would work/read better as a while loop.
/* At this point, any device that had been used by the guest is in
@@ -816,9 +817,25 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr
hostdev_mgr,
* For SRIOV net host devices, unset mac and port profile before
* reset and reattach device
*/
-for (i = 0; i nhostdevs; i++)
-virHostdevNetConfigRestore(hostdevs[i], hostdev_mgr-stateDir,
- oldStateDir);
+for (i = 0; i nhostdevs; i++) {
+virPCIDevicePtr dev;
+virDomainHostdevDefPtr hostdev = hostdevs[i];
+virDomainHostdevSubsysPCIPtr pcisrc =
hostdev-source.subsys.u.pci;
+
+if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
+hostdev-source.subsys.type !=
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI ||
+hostdev-parent.type != VIR_DOMAIN_DEVICE_NET ||
+!hostdev-parent.data.net http://parent.data.net)
This is what makes me believe you sent in html format
It's also directly taken from virHostdevNetConfigRestore... and could be
considered partially from virHostdevPreparePCIDevices
Suggestion - create a patch that has a new static bool function
(isHostdevPCINetDevice) that does the same check passing hostdev...
Then adjust the (currently) two places that make that check in order to
use the bool function to decide to continue or not. Then this patch
would use that function here instead.
+continue;
+
+dev = virPCIDeviceNew(pcisrc-addr.domain, pcisrc-addr.bus,
+ pcisrc-addr.slot, pcisrc-addr.function);
+if (virPCIDeviceListFind(pcidevs, dev)) {
+virHostdevNetConfigRestore(hostdev, hostdev_mgr-stateDir,
+ oldStateDir);
+}
+virPCIDeviceFree(dev);
+ }
It's still not quite clear to me from your description why we have to
search the pcidevs for our device before calling the Restore function.
Does it matter if it's in or not in the activePCIHostdevs list (the
comment just before this section). I think this is one of those cases
where the code perhaps isn't self documenting and that by adding a few
more comments along the way we can better describe what we're trying to
Re: [libvirt] [PATCHv2 3/5] Allocate virtio-serial addresses when starting a domain
On 03/24/2015 07:41 AM, Ján Tomko wrote: On Mon, Mar 23, 2015 at 05:46:23PM -0400, John Ferlan wrote: ... Mostly for my edification... These examples previously although indicating they were auto-assign (of sorts) really weren't - they were more force-assign for each example. Force-assign after this patch? Otherwise I don't understand. I started writing this before figuring out the second part and just didn't go back re-read my original thought... But I think I was trying to point out that the existing code doesn't really auto assign and after your patch these changes are doing that, hence why set to port='0'... I agree with not force-assign, but was just making sure... The way to auto-assign is by setting port='0', correct? Yes. However, I'm still missing something from the *auto.args output. It seems the controller='#' is ignored and I guess I don't understand that... I must've overlooked that. It shouldn't be a problem to take it as a hint in virDomainVirtioSerialAddrAssign. Right so I figured out after I sent this, but figured you'd know that anyway. Sure port='0' (meaning first available port on the controller), but I would have expected to see : kvm.port.0 use virtio.serial.0.0,nr=1... (which it does) kvm.port.foo use virtio.serial.1.0,nr=1... (on controller 0?, but XML has controller='1') kvm.port.bar use virtio.serial.1.0,nr=3... (which it does) kvm.port.wizz use virtio.serial.0.0,nr=2 (incorrect due to others) kvm.port.ooh use virtio.serial.1.0,nr=2, (on controller 0?, but XML has controller='1' kvm.port.lla use virtio.serial.2.0,nr=1 (on controller 0?, but XML has controller='2') Now if been if lla used controller='0', then I assume would nr=4 be chosen since 1,2 were auto-assigned, 3 was specifically assigned, thus 4 would be the next one. Continuing with that same logic, the *-autoassign example could have shown that if controller='0',port='2' and 'controller='1',port='1' were preassigned, then the controllers/ports assigned would be 0.0,nr=1, 0.0,nr=3, 0.0,nr=4, 1.0,nr=2 (since only 4 ports on controller='0' can be used w/ 2 be static and controller='1' having port '1' already in use). nr=4 is out of bounds for a controller with 4 ports. The ports are numbered from 0, but port number 0 can only be used for virtconsoles, not channels. Oh yeah - right...doh! Too much context switching sometimes causes loses of brain to finger functions... John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix can use setmaxmem to change the maximum memory biger than max memory
When call qemuDomainSetMemoryFlags() to change maximum memory size, we
do not check if the maximum memory is biger than the max memory, this will
make vm disappear after libvirtd restart if we set a big maximum memory.
Add a check for this, also fix a typos issue.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_driver.c | 7 +++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d633f93..0d4b165 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16646,7 +16646,7 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
if (src-mem.memory_slots != dst-mem.memory_slots) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(Target domain memory slots count '%u' doesn't match
source '%u),
+ _(Target domain memory slots count '%u' doesn't match
source '%u'),
dst-mem.memory_slots, src-mem.memory_slots);
goto error;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3518dec..86b87af 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2324,6 +2324,13 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
nodes cannot be modified with this API));
goto endjob;
}
+if (persistentDef-mem.max_memory
+persistentDef-mem.max_memory newmem) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(cannot set maximum memory size biger than
+ the max memory size));
+goto endjob;
+}
virDomainDefSetMemoryInitial(persistentDef, newmem);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] qemu: check if domain is really active when do setvcpus with --live
On Tue, Mar 24, 2015 at 02:27:42PM +0800, lhuang wrote:
On 03/20/2015 10:39 PM, Pavel Hrdina wrote:
From: Luyao Huang lhu...@redhat.com
Commit e105dc9 fix setting vcpus for offline domain, but forget check
if vm is active when pass VIR_DOMAIN_AFFECT_LIVE flags.
# virsh setvcpus test3 4 --live
error: Failed to create controller cpu for group: No such file or
directory
Add a check if we pass VIR_DOMAIN_AFFECT_LIVE flags.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204006
Signed-off-by: Luyao Huang lhu...@redhat.com
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_driver.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4942712..c4d96bd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4881,6 +4881,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned
int nvcpus,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup;
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(domain is not running));
+goto endjob;
+}
+}
+
if (flags VIR_DOMAIN_AFFECT_LIVE !(flags
VIR_DOMAIN_VCPU_GUEST)) {
if (virCgroupNewEmulator(priv-cgroup, false, cgroup_temp) 0)
goto endjob;
@@ -4947,12 +4955,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned
int nvcpus,
if (ncpuinfo 0)
goto endjob;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain is not running));
-goto endjob;
-}
-
if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) 0)
goto endjob;
Sorry, i forgot there is a function virDomainLiveConfigHelperMethod(),
move this function just after qemuDomainObjBeginJob() maybe a good way
to fix this issue. Also virDomainLiveConfigHelperMethod() may change
flags, so it should be done more early (as soon as possible after set a
lock to vm).
I've already sent a patch 7/6 to move that function. I realized that right
after I've sent this series to mailing list.
Pavel
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/2] network_conf: check if bridge exists on host for user created bridges
The patch fixes the below problem. == If the bridge name is not mentioned in the network xml, the bridge name is auto generated from virNetworkAllocateBridge(). If the default template named bridge is created manually by a user, the bridge start will fail with File exists. bash-4.3$ sudo brctl addbr virbr1 bash-4.3$ brctl show bridge name bridge id STP enabled interfaces br0 8000. no virbr0 8000.525400a91d03 yes virbr0-nic virbr1 8000. no bash-4.3$ sudo virsh net-list --all Name State Autostart Persistent -- default active noyes bash-4.3$ cat /tmp/isolated # Notice that the bridge intentionally not given. network nameisolated/name forward/ ip address=192.168.123.1 netmask=255.255.255.0 dhcp range start=192.168.123.2 end=192.168.123.254/ /dhcp /ip /network bash-4.3$ sudo virsh net-create /tmp/isolated error: Failed to create network from isolated error: Unable to create bridge virbr1: File exists === --- Shivaprasad G Bhat (2): cleanup conf/device_conf.h inclusion from util/virnetdev.h network_conf: check if bridge exists on host for user created bridges src/conf/device_conf.h | 38 +- src/conf/network_conf.c | 15 --- src/util/virnetdev.h| 38 +- 3 files changed, 50 insertions(+), 41 deletions(-) -- Signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/2] cleanup conf/device_conf.h inclusion from util/virnetdev.h
Move the struct and enum definitions from device_conf.h to
virnetdev.h thus avoiding the file inclusion.
The wrong reference of conf files from util/ is allowed
in Makefile.am for now. The reason being,
The change in Makefile.am for libvirt_util_la_CFLAGS to remove
conf from the utils would require corresponding inclusion in util files
to specify the paths explicitly. This would result in syntax-check failures
(prohibit_cross_inclusion) which need to be worked around until
there are patches reworking the incorrect inclusion.
The syntax-check failure workaround seems dangerous as that might mask some
easily resolvable failures. So dont touch the Makefile.am for now. Resolve
the wrong inclusions in separate patches.
Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
---
src/conf/device_conf.h | 38 +-
src/util/virnetdev.h | 38 +-
2 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index 7ea90f6..a650189 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -31,19 +31,7 @@
# include virutil.h
# include virthread.h
# include virbuffer.h
-
-typedef enum {
-VIR_INTERFACE_STATE_UNKNOWN = 1,
-VIR_INTERFACE_STATE_NOT_PRESENT,
-VIR_INTERFACE_STATE_DOWN,
-VIR_INTERFACE_STATE_LOWER_LAYER_DOWN,
-VIR_INTERFACE_STATE_TESTING,
-VIR_INTERFACE_STATE_DORMANT,
-VIR_INTERFACE_STATE_UP,
-VIR_INTERFACE_STATE_LAST
-} virInterfaceState;
-
-VIR_ENUM_DECL(virInterfaceState)
+# include virnetdev.h
typedef struct _virDevicePCIAddress virDevicePCIAddress;
typedef virDevicePCIAddress *virDevicePCIAddressPtr;
@@ -55,30 +43,6 @@ struct _virDevicePCIAddress {
int multi; /* virTristateSwitch */
};
-typedef struct _virInterfaceLink virInterfaceLink;
-typedef virInterfaceLink *virInterfaceLinkPtr;
-struct _virInterfaceLink {
-virInterfaceState state; /* link state */
-unsigned int speed; /* link speed in Mbits per second */
-};
-
-typedef enum {
-VIR_NET_DEV_FEAT_GRXCSUM,
-VIR_NET_DEV_FEAT_GTXCSUM,
-VIR_NET_DEV_FEAT_GSG,
-VIR_NET_DEV_FEAT_GTSO,
-VIR_NET_DEV_FEAT_GGSO,
-VIR_NET_DEV_FEAT_GGRO,
-VIR_NET_DEV_FEAT_LRO,
-VIR_NET_DEV_FEAT_RXVLAN,
-VIR_NET_DEV_FEAT_TXVLAN,
-VIR_NET_DEV_FEAT_NTUPLE,
-VIR_NET_DEV_FEAT_RXHASH,
-VIR_NET_DEV_FEAT_LAST
-} virNetDevFeature;
-
-VIR_ENUM_DECL(virNetDevFeature)
-
int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr);
int virDevicePCIAddressParseXML(xmlNodePtr node,
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 856127b..daba0bb 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -30,7 +30,6 @@
# include virnetlink.h
# include virmacaddr.h
# include virpci.h
-# include device_conf.h
# ifdef HAVE_STRUCT_IFREQ
typedef struct ifreq virIfreq;
@@ -47,6 +46,43 @@ typedef enum {
} virNetDevRxFilterMode;
VIR_ENUM_DECL(virNetDevRxFilterMode)
+typedef enum {
+VIR_INTERFACE_STATE_UNKNOWN = 1,
+VIR_INTERFACE_STATE_NOT_PRESENT,
+VIR_INTERFACE_STATE_DOWN,
+VIR_INTERFACE_STATE_LOWER_LAYER_DOWN,
+VIR_INTERFACE_STATE_TESTING,
+VIR_INTERFACE_STATE_DORMANT,
+VIR_INTERFACE_STATE_UP,
+VIR_INTERFACE_STATE_LAST
+} virInterfaceState;
+
+VIR_ENUM_DECL(virInterfaceState)
+
+typedef struct _virInterfaceLink virInterfaceLink;
+typedef virInterfaceLink *virInterfaceLinkPtr;
+struct _virInterfaceLink {
+virInterfaceState state; /* link state */
+unsigned int speed; /* link speed in Mbits per second */
+};
+
+typedef enum {
+VIR_NET_DEV_FEAT_GRXCSUM,
+VIR_NET_DEV_FEAT_GTXCSUM,
+VIR_NET_DEV_FEAT_GSG,
+VIR_NET_DEV_FEAT_GTSO,
+VIR_NET_DEV_FEAT_GGSO,
+VIR_NET_DEV_FEAT_GGRO,
+VIR_NET_DEV_FEAT_LRO,
+VIR_NET_DEV_FEAT_RXVLAN,
+VIR_NET_DEV_FEAT_TXVLAN,
+VIR_NET_DEV_FEAT_NTUPLE,
+VIR_NET_DEV_FEAT_RXHASH,
+VIR_NET_DEV_FEAT_LAST
+} virNetDevFeature;
+
+VIR_ENUM_DECL(virNetDevFeature)
+
typedef struct _virNetDevRxFilter virNetDevRxFilter;
typedef virNetDevRxFilter *virNetDevRxFilterPtr;
struct _virNetDevRxFilter {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] GSOC 2015: Libvirt Projects
On 24.03.2015 10:27, Stefan Hajnoczi wrote: On Fri, Mar 13, 2015 at 12:03 AM, Richa Sehgal richasehgal2...@gmail.com wrote: I am currently pursuing Master's in University of Illinois at Urbana Champaign, USA and completed by B.Tech from Indian Institute of Technology - Delhi (IIT- Delhi). I am interested in contributing to Libvirt community through GSOC 2015. I am using VirtualBox to run Ubuntu and have used VMware Player in the past. Actually I am very interested in understanding virtualization at a more practical level and do not want to limit myself to theoretical knowledge. I have played with QEMU during the OS class, but I am new to Libvirt, and thus would like to start with beginner level projects. There are two that I found: 1. Enhancing libvirt-designer 2. Abstracting device address allocation Are these projects still open, or students have already applied to them? Are there any other projects that I can start with? I am really excited about contributing to the libvirt project. Please let me know of a suitable way of starting with these. I look forward for a useful engagement with the community this summer. My irc name is: richashi. Hi, I have CCed the libvirt GSoC mentors in case you haven't contacted them directly yet. She indeed has. We have an ongoing discussion. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: refact virDomainHasDiskMirror and rename it to virDomainHasBlockjob
Create external snapshot or migrate a vm when there is a blockpull
job running should be forbidden by libvirt, otherwise qemu try to
create external snapshot and failed with error unable to execute
QEMU command 'transaction': Device 'drive-virtio-disk0' is busy:
block device is in use by block job: stream, and migration will
succeed which will lead the blockpull job failed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1203628
Signed-off-by: Shanzhi Yu s...@redhat.com
---
src/conf/domain_conf.c| 6 +++---
src/conf/domain_conf.h| 2 +-
src/libvirt_private.syms | 2 +-
src/qemu/qemu_driver.c| 6 +++---
src/qemu/qemu_migration.c | 2 +-
5 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d633f93..24445af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12264,13 +12264,13 @@ virDomainDiskRemoveByName(virDomainDefPtr def, const
char *name)
}
/* Return true if VM has at least one disk involved in a current block
- * copy/commit job (that is, with a mirror element in the disk xml). */
+ * copy/commit/pull job */
bool
-virDomainHasDiskMirror(virDomainObjPtr vm)
+virDomainHasBlockjob(virDomainObjPtr vm)
{
size_t i;
for (i = 0; i vm-def-ndisks; i++)
-if (vm-def-disks[i]-mirror)
+if (vm-def-disks[i]-blockjob)
return true;
return false;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bceb2d7..32674f3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2645,7 +2645,7 @@ int virDomainDiskSourceParse(xmlNodePtr node,
xmlXPathContextPtr ctxt,
virStorageSourcePtr src);
-bool virDomainHasDiskMirror(virDomainObjPtr vm);
+bool virDomainHasBlockjob(virDomainObjPtr vm);
int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net);
virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 33222f0..9ebaf4a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -297,7 +297,7 @@ virDomainGraphicsTypeFromString;
virDomainGraphicsTypeToString;
virDomainGraphicsVNCSharePolicyTypeFromString;
virDomainGraphicsVNCSharePolicyTypeToString;
-virDomainHasDiskMirror;
+virDomainHasBlockjob;
virDomainHasNet;
virDomainHostdevCapsTypeToString;
virDomainHostdevDefAlloc;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 80a3d77..51e302f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7398,7 +7398,7 @@ static virDomainPtr
qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml
virObjectRef(vm);
def = NULL;
-if (virDomainHasDiskMirror(vm)) {
+if (virDomainHasBlockjob(vm)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s,
_(domain has active block job));
virDomainObjAssignDef(vm, NULL, false, NULL);
@@ -14583,7 +14583,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
%s, _(domain is marked for auto destroy));
goto cleanup;
}
-if (virDomainHasDiskMirror(vm)) {
+if (virDomainHasBlockjob(vm)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s,
_(domain has active block job));
goto cleanup;
@@ -15245,7 +15245,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
-if (virDomainHasDiskMirror(vm)) {
+if (virDomainHasBlockjob(vm)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s,
_(domain has active block job));
goto cleanup;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d34bb02..27a76ec 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1977,7 +1977,7 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver,
virDomainObjPtr vm,
}
-if (virDomainHasDiskMirror(vm)) {
+if (virDomainHasBlockjob(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(domain has an active block job));
return false;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/7] Introduce storage pool status XML
All of the commits resolve BZ https://bugzilla.redhat.com/show_bug.cgi?id=1177733 Erik Skultety (7): storage: Remove unused attribute conn from 'checkPool' callback conf: Add support for storage state directory conf: Add/modify storage formatting functions storage: Modify stateInitialize to support storage state XML conf: Introduce virStoragePoolLoadAllState virStoragePoolLoadState storage: Introduce storagePoolUpdateAllState function storage: Create/Delete pool status XML src/conf/storage_conf.c | 229 -- src/conf/storage_conf.h | 14 ++- src/libvirt_private.syms | 2 + src/storage/storage_backend.h | 3 +- src/storage/storage_backend_fs.c | 3 +- src/storage/storage_backend_iscsi.c | 3 +- src/storage/storage_backend_logical.c | 3 +- src/storage/storage_backend_mpath.c | 3 +- src/storage/storage_backend_scsi.c| 3 +- src/storage/storage_backend_zfs.c | 3 +- src/storage/storage_driver.c | 166 +++- 11 files changed, 348 insertions(+), 84 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] conf: Add support for storage state directory
Before introducing necessary changes to storage_driver.c, first prepare
our structures for storage state XML support.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/conf/storage_conf.h | 1 +
src/storage/storage_driver.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 4584075..8ccc947 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -293,6 +293,7 @@ struct _virStorageDriverState {
char *configDir;
char *autostartDir;
+char *stateDir;
bool privileged;
};
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 64ea770..e088ffa 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -261,6 +261,7 @@ storageStateCleanup(void)
VIR_FREE(driver-configDir);
VIR_FREE(driver-autostartDir);
+VIR_FREE(driver-stateDir);
storageDriverUnlock();
virMutexDestroy(driver-lock);
VIR_FREE(driver);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] storage: Remove unused attribute conn from 'checkPool' callback
The checkPool callback should be used when updating states of all pools
during storageStateInitialize, not storageDriverAutostart, otherwise we
can't start a domain which mounts a volume after daemons restarted. This
is because qemuProcessReconnect is called earlier than
storageDriverAutostart which marks the pool as active, so that the
domain can mount a volume from this pool successfully.
In order to fix this, conn attribute has to be discarded from the
callback, because storageStateInitialize doesn't have a connection yet.
(it's not used anyway)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/storage/storage_backend.h | 3 +--
src/storage/storage_backend_fs.c | 3 +--
src/storage/storage_backend_iscsi.c | 3 +--
src/storage/storage_backend_logical.c | 3 +--
src/storage/storage_backend_mpath.c | 3 +--
src/storage/storage_backend_scsi.c| 3 +--
src/storage/storage_backend_zfs.c | 3 +--
src/storage/storage_driver.c | 2 +-
8 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 9f1db60..fd2451c 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -34,8 +34,7 @@
typedef char * (*virStorageBackendFindPoolSources)(virConnectPtr conn,
const char *srcSpec,
unsigned int flags);
-typedef int (*virStorageBackendCheckPool)(virConnectPtr conn,
- virStoragePoolObjPtr pool,
+typedef int (*virStorageBackendCheckPool)(virStoragePoolObjPtr pool,
bool *active);
typedef int (*virStorageBackendStartPool)(virConnectPtr conn,
virStoragePoolObjPtr pool);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 35385db..d4d65bc 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -533,8 +533,7 @@ virStorageBackendFileSystemUnmount(virStoragePoolObjPtr
pool)
static int
-virStorageBackendFileSystemCheck(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool,
+virStorageBackendFileSystemCheck(virStoragePoolObjPtr pool,
bool *isActive)
{
if (pool-def-type == VIR_STORAGE_POOL_DIR) {
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
index 079c767..497a71b 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -236,8 +236,7 @@ virStorageBackendISCSIFindPoolSources(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
static int
-virStorageBackendISCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
-virStoragePoolObjPtr pool,
+virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool,
bool *isActive)
{
char *session = NULL;
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 7ba8ded..11c5683 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -479,8 +479,7 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn
ATTRIBUTE_UNUSED,
static int
-virStorageBackendLogicalCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool,
+virStorageBackendLogicalCheckPool(virStoragePoolObjPtr pool,
bool *isActive)
{
*isActive = virFileExists(pool-def-target.path);
diff --git a/src/storage/storage_backend_mpath.c
b/src/storage/storage_backend_mpath.c
index 44bcd60..971408a 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -245,8 +245,7 @@ virStorageBackendGetMaps(virStoragePoolObjPtr pool)
}
static int
-virStorageBackendMpathCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
-virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+virStorageBackendMpathCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
bool *isActive)
{
*isActive = virFileExists(/dev/mpath);
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index 58e7e6d..66e0846 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -844,8 +844,7 @@ deleteVport(virConnectPtr conn,
static int
-virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool,
+virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool,
bool *isActive)
{
char *path = NULL;
diff --git a/src/storage/storage_backend_zfs.c
b/src/storage/storage_backend_zfs.c
index 9482706..cb2662a 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -41,8
Re: [libvirt] GSOC 2015: Libvirt Projects
On Fri, Mar 13, 2015 at 12:03 AM, Richa Sehgal richasehgal2...@gmail.com wrote: I am currently pursuing Master's in University of Illinois at Urbana Champaign, USA and completed by B.Tech from Indian Institute of Technology - Delhi (IIT- Delhi). I am interested in contributing to Libvirt community through GSOC 2015. I am using VirtualBox to run Ubuntu and have used VMware Player in the past. Actually I am very interested in understanding virtualization at a more practical level and do not want to limit myself to theoretical knowledge. I have played with QEMU during the OS class, but I am new to Libvirt, and thus would like to start with beginner level projects. There are two that I found: 1. Enhancing libvirt-designer 2. Abstracting device address allocation Are these projects still open, or students have already applied to them? Are there any other projects that I can start with? I am really excited about contributing to the libvirt project. Please let me know of a suitable way of starting with these. I look forward for a useful engagement with the community this summer. My irc name is: richashi. Hi, I have CCed the libvirt GSoC mentors in case you haven't contacted them directly yet. All project ideas are open until the student application deadline on March 27th. You need to have the right skills for the project whether or not there are other candidates applying, so I suggest focussing on the project ideas that most interest you. Good luck! Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] conf: Add/modify storage formatting functions
This patch introduces virStoragePoolSaveStatus to properly format the
status XML. It also adds virStoragePoolDefFormatBuf function, which
alike in the network driver, formats the whole storage pool definition into
a buffer that might have been previously modified by
virStoragePoolSaveStatus to insert poolstatus element. The original
virStoragePoolDefFormat function had to be modified accordingly to use
virBuffer.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/conf/storage_conf.c | 139 ++-
src/conf/storage_conf.h | 6 +-
src/libvirt_private.syms | 1 +
3 files changed, 107 insertions(+), 39 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index b070448..5d984f3 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1150,76 +1150,90 @@ virStoragePoolSourceFormat(virBufferPtr buf,
}
-char *
-virStoragePoolDefFormat(virStoragePoolDefPtr def)
+int
+virStoragePoolDefFormatBuf(virBufferPtr buf,
+ virStoragePoolDefPtr def)
{
virStoragePoolOptionsPtr options;
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-const char *type;
char uuid[VIR_UUID_STRING_BUFLEN];
+const char *type;
options = virStoragePoolOptionsForPoolType(def-type);
if (options == NULL)
-return NULL;
+goto error;
type = virStoragePoolTypeToString(def-type);
if (!type) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(unexpected pool type));
-goto cleanup;
+goto error;
}
-virBufferAsprintf(buf, pool type='%s'\n, type);
-virBufferAdjustIndent(buf, 2);
-virBufferEscapeString(buf, name%s/name\n, def-name);
+virBufferAsprintf(buf, pool type='%s'\n, type);
+virBufferAdjustIndent(buf, 2);
+virBufferEscapeString(buf, name%s/name\n, def-name);
virUUIDFormat(def-uuid, uuid);
-virBufferAsprintf(buf, uuid%s/uuid\n, uuid);
+virBufferAsprintf(buf, uuid%s/uuid\n, uuid);
-virBufferAsprintf(buf, capacity unit='bytes'%llu/capacity\n,
+virBufferAsprintf(buf, capacity unit='bytes'%llu/capacity\n,
def-capacity);
-virBufferAsprintf(buf, allocation unit='bytes'%llu/allocation\n,
+virBufferAsprintf(buf, allocation unit='bytes'%llu/allocation\n,
def-allocation);
-virBufferAsprintf(buf, available unit='bytes'%llu/available\n,
+virBufferAsprintf(buf, available unit='bytes'%llu/available\n,
def-available);
-if (virStoragePoolSourceFormat(buf, options, def-source) 0)
-goto cleanup;
+if (virStoragePoolSourceFormat(buf, options, def-source) 0)
+goto error;
/* RBD, Sheepdog, and Gluster devices are not local block devs nor
* files, so they don't have a target */
if (def-type != VIR_STORAGE_POOL_RBD
def-type != VIR_STORAGE_POOL_SHEEPDOG
def-type != VIR_STORAGE_POOL_GLUSTER) {
-virBufferAddLit(buf, target\n);
-virBufferAdjustIndent(buf, 2);
+virBufferAddLit(buf, target\n);
+virBufferAdjustIndent(buf, 2);
-virBufferEscapeString(buf, path%s/path\n, def-target.path);
+virBufferEscapeString(buf, path%s/path\n, def-target.path);
-virBufferAddLit(buf, permissions\n);
-virBufferAdjustIndent(buf, 2);
-virBufferAsprintf(buf, mode0%o/mode\n,
+virBufferAddLit(buf, permissions\n);
+virBufferAdjustIndent(buf, 2);
+virBufferAsprintf(buf, mode0%o/mode\n,
def-target.perms.mode);
-virBufferAsprintf(buf, owner%d/owner\n,
+virBufferAsprintf(buf, owner%d/owner\n,
(int) def-target.perms.uid);
-virBufferAsprintf(buf, group%d/group\n,
+virBufferAsprintf(buf, group%d/group\n,
(int) def-target.perms.gid);
-virBufferEscapeString(buf, label%s/label\n,
+virBufferEscapeString(buf, label%s/label\n,
def-target.perms.label);
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, /permissions\n);
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, /target\n);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, /permissions\n);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, /target\n);
}
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, /pool\n);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, /pool\n);
+
+return 0;
+
+ error:
+return -1;
+}
+
+char *
+virStoragePoolDefFormat(virStoragePoolDefPtr def)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+if (virStoragePoolDefFormatBuf(buf, def) 0)
+goto error;
if (virBufferCheckError(buf) 0)
-goto cleanup;
+goto error;
return virBufferContentAndReset(buf);
- cleanup:
+ error:
[libvirt] [PATCH 6/7] storage: Introduce storagePoolUpdateAllState function
The update was originally part of the storageDriverAutostart function,
but the pools have to be checked earlier during initialization phase, so
the 'checkPool' block has been moved to storagePoolUpdateAllState.
Prior to this update virStoragePoolLoadAllConfigs and
virStoragePoolLoadAllState functions gather all existing pool in the
system, so first it is necessary to filter out the ones that are
inactive (only config XML found), then determine storage backends for
the rest of the pools and run checkPool on each one of them to update
their 'active' property.
After update, pools have to be refreshed, otherwise the list of volumes
stays empty. Once again we need the connection, but all the
storage backends ignore this argument except for RBD, however RBD
doesn't support 'checkPool' callback, therefore it is safe to pass
connection as NULL pointer.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/storage/storage_driver.c | 73
1 file changed, 61 insertions(+), 12 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index d09acce..2899521 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -75,6 +75,64 @@ static void storageDriverUnlock(void)
}
static void
+storagePoolUpdateAllState(void)
+{
+size_t i;
+bool active = false;
+
+for (i = 0; i driver-pools.count; i++) {
+virStoragePoolObjPtr pool = driver-pools.objs[i];
+virStorageBackendPtr backend;
+
+virStoragePoolObjLock(pool);
+if (!virStoragePoolObjIsActive(pool)) {
+virStoragePoolObjUnlock(pool);
+continue;
+}
+
+if ((backend = virStorageBackendForType(pool-def-type)) == NULL) {
+VIR_ERROR(_(Missing backend %d), pool-def-type);
+virStoragePoolObjUnlock(pool);
+continue;
+}
+
+/* Backends which do not support 'checkPool' are considered
+ * inactive by default.
+ */
+if (backend-checkPool
+backend-checkPool(pool, active) 0) {
+virErrorPtr err = virGetLastError();
+VIR_ERROR(_(Failed to initialize storage pool '%s': %s),
+ pool-def-name, err ? err-message :
+ _(no error message found));
+virStoragePoolObjUnlock(pool);
+continue;
+}
+
+/* We can pass NULL as connection, most backends do not use
+ * it anyway, but if they do and fail, we want to log error and
+ * continue with other pools.
+ */
+if (active) {
+virStoragePoolObjClearVols(pool);
+if (backend-refreshPool(NULL, pool) 0) {
+virErrorPtr err = virGetLastError();
+if (backend-stopPool)
+backend-stopPool(NULL, pool);
+VIR_ERROR(_(Failed to restart storage pool '%s': %s),
+ pool-def-name, err ? err-message :
+ _(no error message found));
+virStoragePoolObjUnlock(pool);
+continue;
+}
+}
+
+pool-active = active;
+virStoragePoolObjUnlock(pool);
+}
+}
+
+static void
storageDriverAutostart(void)
{
size_t i;
@@ -99,18 +157,7 @@ storageDriverAutostart(void)
continue;
}
-if (backend-checkPool
-backend-checkPool(pool, started) 0) {
-virErrorPtr err = virGetLastError();
-VIR_ERROR(_(Failed to initialize storage pool '%s': %s),
- pool-def-name, err ? err-message :
- _(no error message found));
-virStoragePoolObjUnlock(pool);
-continue;
-}
-
-if (!started
-pool-autostart
+if (pool-autostart
!virStoragePoolObjIsActive(pool)) {
if (backend-startPool
backend-startPool(conn, pool) 0) {
@@ -207,6 +254,8 @@ storageStateInitialize(bool privileged,
driver-autostartDir) 0)
goto error;
+storagePoolUpdateAllState();
+
storageDriverUnlock();
ret = 0;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/5] Allocate virtio-serial addresses when starting a domain
On Mon, Mar 23, 2015 at 05:46:23PM -0400, John Ferlan wrote:
On 03/17/2015 07:41 AM, Ján Tomko wrote:
Instead of always using controller 0 and incrementing port number,
respect the maximum port numbers of controllers and use all of them.
Ports for virtio consoles are quietly reserved, but not formatted
(neither in XML nor on QEMU command line).
Also rejects duplicate virtio-serial addresses.
https://bugzilla.redhat.com/show_bug.cgi?id=890606
https://bugzilla.redhat.com/show_bug.cgi?id=1076708
---
src/conf/domain_conf.c | 29 --
src/qemu/qemu_command.c| 63
++
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_process.c| 2 +
.../qemuxml2argv-channel-virtio-auto.args | 8 +--
.../qemuxml2argv-channel-virtio-autoassign.args| 10 ++--
.../qemuxml2xmlout-channel-virtio-auto.xml | 10 ++--
8 files changed, 81 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 02105c3..e7f86e1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1399,6 +1399,65 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def,
virDomainDeviceInfoPtr info,
return 0;
}
+
+static int
+qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def,
+ virDomainObjPtr obj)
+{
+int ret = -1;
+size_t i;
+virDomainVirtioSerialAddrSetPtr addrs = NULL;
+qemuDomainObjPrivatePtr priv = NULL;
+
+if (!(addrs = virDomainVirtioSerialAddrSetCreate()))
+goto cleanup;
Coverity determines addrs != NULL, but
+
+if (virDomainVirtioSerialAddrSetAddControllers(addrs, def) 0)
+goto cleanup;
+
+if (virDomainDeviceInfoIterate(def, virDomainVirtioSerialAddrReserve,
+ addrs) 0)
+goto cleanup;
+
+VIR_DEBUG(Finished reserving existing ports);
+
+for (i = 0; i def-nconsoles; i++) {
+virDomainChrDefPtr chr = def-consoles[i];
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE
+chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) {
+if (virDomainVirtioSerialAddrAssign(addrs, chr-info, true)
0)
+goto cleanup;
+}
+}
+
+for (i = 0; i def-nchannels; i++) {
+virDomainChrDefPtr chr = def-channels[i];
+if (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
+chr-info.addr.vioserial.port == 0
+virDomainVirtioSerialAddrAssign(addrs, chr-info, false) 0)
+goto cleanup;
+}
+
+if (obj obj-privateData) {
+priv = obj-privateData;
+if (addrs) {
There's a check here where the else is DEADCODE
Right. The address set should only be generated if there is a
virtio-serial controller present.
+/* if this is the live domain object, we persist the addresses
*/
+virDomainVirtioSerialAddrSetFree(priv-vioserialaddrs);
+priv-persistentAddrs = 1;
+priv-vioserialaddrs = addrs;
+addrs = NULL;
+} else {
+priv-persistentAddrs = 0;
+}
+}
+ret = 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ae315df..5589f22 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5205,6 +5205,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
virDomainDefClearCCWAddresses(vm-def);
virDomainCCWAddressSetFree(priv-ccwaddrs);
priv-ccwaddrs = NULL;
+virDomainVirtioSerialAddrSetFree(priv-vioserialaddrs);
+priv-vioserialaddrs = NULL;
}
qemuDomainReAttachHostDevices(driver, vm-def);
Mostly for my edification... These examples previously although
indicating they were auto-assign (of sorts) really weren't - they were
more force-assign for each example.
Force-assign after this patch? Otherwise I don't understand.
The way to auto-assign is by setting port='0', correct?
Yes.
However, I'm still missing something from the *auto.args output. It
seems the controller='#' is ignored and I guess I don't understand
that...
I must've overlooked that.
It shouldn't be a problem to take it as a hint in
virDomainVirtioSerialAddrAssign.
Sure port='0' (meaning first available port on the
controller), but I would have expected to see :
kvm.port.0 use virtio.serial.0.0,nr=1... (which it does)
kvm.port.foo use virtio.serial.1.0,nr=1... (on controller 0?, but XML
has controller='1')
kvm.port.bar use virtio.serial.1.0,nr=3... (which it does)
kvm.port.wizz use virtio.serial.0.0,nr=2 (incorrect due to others)
kvm.port.ooh use
[libvirt] [PATCH v3 2/2] network_conf: check if bridge exists on host for user created bridges
virNetworkBridgeInUse() doesn't check if the bridge is manually created
outside of libvirt. Check if the bridge actually exist on host using the
virNetDevExists().
Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
---
src/conf/network_conf.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d7c5dec..c3ae2e4 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3227,13 +3227,22 @@ virNetworkBridgeInUseHelper(const void *payload,
int ret = 0;
virNetworkObjPtr net = (virNetworkObjPtr) payload;
const struct virNetworkBridgeInUseHelperData *data = opaque;
+bool defined_bridge = false;
virObjectLock(net);
if (net-def-bridge
-STREQ(net-def-bridge, data-bridge)
-!(data-skipname STREQ(net-def-name, data-skipname)))
-ret = 1;
+STREQ(net-def-bridge, data-bridge)) {
+defined_bridge = true;
+if (!(data-skipname STREQ(net-def-name, data-skipname)))
+ ret = 1;
+}
+
virObjectUnlock(net);
+
+/* Bridge might have been created by a user manually outside libvirt */
+if (!defined_bridge !ret)
+ret = virNetDevExists(data-bridge) ? 1 : 0;
+
return ret;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/7] storage: Create/Delete pool status XML
Once we introduced virStoragePoolSaveStatus function, create a status
XML every time a pool is created (virStoragePoolCreate,
storageDriverAutostart)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/storage/storage_driver.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 2899521..4ce3d34 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -136,6 +136,7 @@ static void
storageDriverAutostart(void)
{
size_t i;
+char *stateFile = NULL;
virConnectPtr conn = NULL;
/* XXX Remove hardcoding of QEMU URI */
@@ -183,6 +184,12 @@ storageDriverAutostart(void)
virStoragePoolObjUnlock(pool);
continue;
}
+
+if (!(stateFile = virFileBuildPath(driver-stateDir,
+ pool-def-name, .xml)))
+continue;
+
+ignore_value(virStoragePoolSaveStatus(stateFile, pool-def));
pool-active = 1;
}
virStoragePoolObjUnlock(pool);
@@ -812,6 +819,7 @@ storagePoolCreate(virStoragePoolPtr obj,
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
int ret = -1;
+char *stateFile = NULL;
virCheckFlags(0, -1);
@@ -840,11 +848,21 @@ storagePoolCreate(virStoragePoolPtr obj,
goto cleanup;
}
+/* save pool state */
+if (!(stateFile = virFileBuildPath(driver-stateDir,
+ pool-def-name, .xml)))
+goto cleanup;
+
+if ((ret = virStoragePoolSaveStatus(stateFile,
+pool-def)) 0)
+goto cleanup;
+
VIR_INFO(Starting up storage pool '%s', pool-def-name);
pool-active = 1;
ret = 0;
cleanup:
+VIR_FREE(stateFile);
virStoragePoolObjUnlock(pool);
return ret;
}
@@ -889,6 +907,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
{
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
+char *stateFile = NULL;
int ret = -1;
storageDriverLock();
@@ -937,6 +956,15 @@ storagePoolDestroy(virStoragePoolPtr obj)
pool-def = pool-newDef;
pool-newDef = NULL;
}
+
+if (!(stateFile = virFileBuildPath(driver-stateDir,
+ pool-def-name,
+ .xml)))
+goto cleanup;
+
+unlink(stateFile);
+VIR_FREE(stateFile);
+
ret = 0;
cleanup:
@@ -952,6 +980,7 @@ storagePoolDelete(virStoragePoolPtr obj,
{
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
+char *stateFile = NULL;
int ret = -1;
if (!(pool = virStoragePoolObjFromStoragePool(obj)))
@@ -985,6 +1014,14 @@ storagePoolDelete(virStoragePoolPtr obj,
if (backend-deletePool(obj-conn, pool, flags) 0)
goto cleanup;
VIR_INFO(Deleting storage pool '%s', pool-def-name);
+
+if (!(stateFile = virFileBuildPath(driver-stateDir,
+ pool-def-name,
+ .xml)))
+goto cleanup;
+
+unlink(stateFile);
+VIR_FREE(stateFile);
ret = 0;
cleanup:
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] storage: Modify stateInitialize to support storage state XML
Storage state driver directories initialization needs to be modified
to become more generic.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/storage/storage_driver.c | 52
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index e088ffa..9bd93d2 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -154,54 +154,60 @@ storageStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
void *opaque ATTRIBUTE_UNUSED)
{
-char *base = NULL;
+int ret = -1;
+char *configdir = NULL;
+char *rundir = NULL;
if (VIR_ALLOC(driver) 0)
-return -1;
+return ret;
if (virMutexInit(driver-lock) 0) {
VIR_FREE(driver);
-return -1;
+return ret;
}
storageDriverLock();
if (privileged) {
-if (VIR_STRDUP(base, SYSCONFDIR /libvirt) 0)
+if (VIR_STRDUP(driver-configDir,
+ SYSCONFDIR /libvirt/storage) 0 ||
+VIR_STRDUP(driver-autostartDir,
+ SYSCONFDIR /libvirt/storage/autostart) 0 ||
+VIR_STRDUP(driver-stateDir,
+ LOCALSTATEDIR /run/libvirt/storage) 0)
goto error;
} else {
-base = virGetUserConfigDirectory();
-if (!base)
+configdir = virGetUserConfigDirectory();
+rundir = virGetUserRuntimeDirectory();
+if (!(configdir rundir))
+goto error;
+
+if ((virAsprintf(driver-configDir,
+%s/storage, configdir) 0) ||
+(virAsprintf(driver-autostartDir,
+%s/storage, configdir) 0) ||
+(virAsprintf(driver-stateDir,
+ %s/storage/run, rundir) 0))
goto error;
}
driver-privileged = privileged;
-/*
- * Configuration paths are either $USER_CONFIG_HOME/libvirt/storage/...
- * (session) or /etc/libvirt/storage/... (system).
- */
-if (virAsprintf(driver-configDir,
-%s/storage, base) == -1)
-goto error;
-
-if (virAsprintf(driver-autostartDir,
-%s/storage/autostart, base) == -1)
-goto error;
-
-VIR_FREE(base);
-
if (virStoragePoolLoadAllConfigs(driver-pools,
driver-configDir,
driver-autostartDir) 0)
goto error;
storageDriverUnlock();
-return 0;
+
+ret = 0;
+ cleanup:
+VIR_FREE(configdir);
+VIR_FREE(rundir);
+return ret;
error:
-VIR_FREE(base);
storageDriverUnlock();
storageStateCleanup();
-return -1;
+goto cleanup;
}
/**
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] conf: Introduce virStoragePoolLoadAllState virStoragePoolLoadState
These functions operate exactly the same as
virStoragePoolLoadAllConfigs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/conf/storage_conf.c | 90
src/conf/storage_conf.h | 7
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 11 ++
4 files changed, 109 insertions(+)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 5d984f3..b158e30 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1863,6 +1863,96 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
}
+virStoragePoolObjPtr
+virStoragePoolLoadState(virStoragePoolObjListPtr pools,
+const char *stateDir,
+const char *name)
+{
+char *stateFile = NULL;
+virStoragePoolDefPtr def = NULL;
+virStoragePoolObjPtr pool = NULL;
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
+xmlNodePtr node = NULL;
+
+if (!(stateFile = virFileBuildPath(stateDir, name, .xml)))
+goto cleanup;
+
+if (!(xml = virXMLParseCtxt(stateFile, NULL, _((pool status)), ctxt)))
+goto cleanup;
+
+if (!(node = virXPathNode(//pool, ctxt))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Could not find any 'pool' element in status file));
+goto cleanup;
+}
+
+ctxt-node = node;
+if (!(def = virStoragePoolDefParseXML(ctxt)))
+goto cleanup;
+
+if (!STREQ(name, def-name)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Storage pool status file '%s' does not match
+ pool name '%s'),
+ stateFile, def-name);
+goto cleanup;
+}
+
+/* create the object */
+if (!(pool = virStoragePoolObjAssignDef(pools, def)))
+goto cleanup;
+
+/* XXX: future handling of some additional usefull status data,
+ * for now, if a status file for a pool exists, the pool will be marked
+ * as active
+ */
+
+pool-active = 1;
+
+ cleanup:
+VIR_FREE(stateFile);
+xmlFree(xml);
+xmlXPathFreeContext(ctxt);
+return pool;
+}
+
+
+int
+virStoragePoolLoadAllState(virStoragePoolObjListPtr pools,
+ const char *stateDir)
+{
+DIR *dir;
+struct dirent *entry;
+int ret = -1;
+
+if (!(dir = opendir(stateDir))) {
+if (errno == ENOENT)
+return 0;
+
+virReportSystemError(errno, _(Failed to open dir '%s'), stateDir);
+return -1;
+}
+
+while ((ret = virDirRead(dir, entry, stateDir)) 0) {
+virStoragePoolObjPtr pool;
+
+if (entry-d_name[0] == '.')
+continue;
+
+if (!virFileStripSuffix(entry-d_name, .xml))
+continue;
+
+if (!(pool = virStoragePoolLoadState(pools, stateDir, entry-d_name)))
+continue;
+virStoragePoolObjUnlock(pool);
+}
+
+closedir(dir);
+return ret;
+}
+
+
int
virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
const char *configDir,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 99b2f4a..1f84504 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -318,6 +318,13 @@ int virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr
pools,
const char *configDir,
const char *autostartDir);
+int virStoragePoolLoadAllState(virStoragePoolObjListPtr pools,
+ const char *stateDir);
+
+virStoragePoolObjPtr
+virStoragePoolLoadState(virStoragePoolObjListPtr pools,
+const char *stateDir,
+const char *name);
virStoragePoolObjPtr
virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools,
const unsigned char *uuid);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 689a08f..9bc8de8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -798,6 +798,7 @@ virStoragePoolFormatFileSystemNetTypeToString;
virStoragePoolFormatFileSystemTypeToString;
virStoragePoolGetVhbaSCSIHostParent;
virStoragePoolLoadAllConfigs;
+virStoragePoolLoadAllState;
virStoragePoolObjAssignDef;
virStoragePoolObjClearVols;
virStoragePoolObjDeleteDef;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 9bd93d2..d09acce 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -191,6 +191,17 @@ storageStateInitialize(bool privileged,
}
driver-privileged = privileged;
+if (virFileMakePath(driver-stateDir) 0) {
+virReportError(errno,
+ _(cannot create directory %s),
+ driver-stateDir);
+goto error;
+}
+
+if (virStoragePoolLoadAllState(driver-pools,
+ driver-stateDir) 0)
+goto error;
+
if
Re: [libvirt] [PATCH 00/10] libxl: switch driver to use a single libxl_ctx
Michal Privoznik wrote: On 18.02.2015 04:56, Jim Fehlig wrote: This series is a follow up to https://www.redhat.com/archives/libvir-list/2015-February/msg00024.html It goes a step further and changes the libxl driver to use one, driver-wide libxl_ctx. Currently the libxl driver has one driver-wide ctx for operations that are not domain-specific and a ctx for each domain. This approach was necessary back in the old Xen4.1 libxl days, but with the newer libxl it is more of a hinderance than benefit. Ian Jackson suggested moving to a single ctx while discussing some deadlocks and assertions encountered in the libxl driver when under load from tests such as OpenStack Tempest. Making such a change involves quite a bit of code movement. I've tried to split that up into a reviewable series, the result of which are the 9 patches that follow. I've ran this through all of my automated tests as well as some hacky tests I created to reproduce failures revealed by Tempest. One downside of moving to a single ctx is losing the per-domain log files. Currently, a single log stream can be associated with ctx, hence all logging from libxl will go to a single file. Ian is going to investigate possibilities to accommodate per-domain log files in libxl, but in the meantime folks using Xen are accustomed to a single log file from the xend days. I've been testing this series on xen-unstable and Xen 4.4.1 + commits 2ffeb5d7, 4b9143e4, 5a968257, 60ce518a, 66bff9fd, 77a1bf37, f49f9b41, 6b5a5bba, 93699882d, f1335f0d, and 8bc64413. Results are much better than before applying the series, but I do notice a stuck hypercall after many (hundreds) concurrent domain create/destroy operations. The single libxl_ctx is locked in the callpath, essentially deadlocking the driver. Thread 1 (Thread 0x7f0649a198c0 (LWP 2235)): 0 0x7f0645272397 in ioctl () from /lib64/libc.so.6 1 0x7f0645d8e353 in linux_privcmd_hypercall (xch=optimized out, h=optimized out, hypercall=optimized out) at xc_linux_osdep.c:134 2 0x7f0645d854b8 in do_xen_hypercall (xch=xch@entry=0x7f0630039390, hypercall=hypercall@entry=0x7fffd53f80e0) at xc_private.c:249 3 0x7f0645d86aa4 in do_sysctl (sysctl=sysctl@entry=0x7fffd53f8080, xch=xch@entry=0x7f0630039390) at xc_private.h:281 4 xc_sysctl (xch=xch@entry=0x7f0630039390, sysctl=sysctl@entry=0x7fffd53f8170) at xc_private.c:656 5 0x7f0645d7bfbf in xc_domain_getinfolist (xch=0x7f0630039390, first_domain=first_domain@entry=119, max_domains=max_domains@entry=1, info=info@entry=0x7fffd53f8260) at xc_domain.c:382 6 0x7f0645fabca6 in domain_death_xswatch_callback (egc=0x7fffd53f83f0, w=optimized out, wpath=optimized out, epath=optimized out) at libxl.c:1041 7 0x7f0645fd75a8 in watchfd_callback (egc=0x7fffd53f83f0, ev=optimized out, fd=optimized out, events=optimized out, revents=optimized out) at libxl_event.c:515 8 0x7f0645fd8ac3 in libxl_osevent_occurred_fd (ctx=optimized out, for_libxl=optimized out, fd=optimized out, events_ign=optimized out, revents_ign=optimized out) at libxl_event.c:1259 9 0x7f063a23402c in libxlFDEventCallback (watch=454, fd=33, vir_events=1, fd_info=0x7f0608007e70) at libxl/libxl_driver.c:123 There is no hint in any logs or dmesg suggesting a cause for the stuck hypercall. Any suggestions for further debugging tips appreciated. FYI, this was not a hung hypercall, but looping clear back in frame 6 that I overlooked. It was fixed in libxl by the following commit http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=4783c99aab866f470bd59368cfbf5ad5f677b0ec Jim Fehlig (10): libxl: remove redundant calls to libxl_evdisable_domain_death libxl: use libxl_ctx passed to libxlConsoleCallback libxl: use driver-wide ctx in fd and timer event handling libxl: Move setup of child processing code to driver initialization libxl: move event registration to driver initialization libxl: use global libxl_ctx in event handler libxl: remove unnecessary libxlDomainEventsRegister libxl: make libxlDomainFreeMem static libxl: remove per-domain libxl_ctx libxl: change libxl log stream to ERROR log level src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_domain.c| 438 ++- src/libxl/libxl_domain.h| 27 +-- src/libxl/libxl_driver.c| 484 +++- src/libxl/libxl_migration.c | 17 +- 5 files changed, 426 insertions(+), 542 deletions(-) ACK series Thanks! 1 and 2 were pushed earlier as part of this trivial series https://www.redhat.com/archives/libvir-list/2015-March/msg00102.html I've now pushed 3-9, but held off on pushing 10 since it removes the possibility to get debug level messages from libxl. I think a better approach would be to introduce /etc/libvirt/libxl.conf with a 'log_level' setting,
Re: [libvirt] [PATCH 1/4] util: netlink function to delete any network device
On 03/23/2015 03:43 PM, Laine Stump wrote:
libvirt has always used the netlink RTM_DELLINK message to delete
macvtap/macvlan devices, but it can actually be used to delete other
types of network devices, such as bonds and bridges. This patch makes
virNetDevMacVLanDelete() available as a generic function so it can
intelligibly be called to delete these other types of interfaces.
---
src/libvirt_private.syms | 1 +
src/util/virnetlink.c| 89
src/util/virnetlink.h| 3 +-
3 files changed, 92 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ca3520d..631edf3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1805,6 +1805,7 @@ virNetDevVPortProfileOpTypeToString;
# util/virnetlink.h
virNetlinkCommand;
+virNetlinkDelLink;
virNetlinkEventAddClient;
virNetlinkEventRemoveClient;
virNetlinkEventServiceIsRunning;
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index d52f66a..86c9c9c 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -277,6 +277,87 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
}
+/**
+ * virNetlinkDelLink:
+ *
+ * @ifname: Name of the link
+ *
+ * delete a network link (aka interface aka device) with the given
+ * name. This works for many different types of network devices,
+ * including macvtap and bridges.
+ *
+ * Returns 0 on success, -1 on fatal error.
+ */
+int
+virNetlinkDelLink(const char *ifname)
+{
+int rc = -1;
+struct nlmsghdr *resp = NULL;
+struct nlmsgerr *err;
+struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
+unsigned int recvbuflen;
+struct nl_msg *nl_msg;
+
+nl_msg = nlmsg_alloc_simple(RTM_DELLINK,
+NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
+if (!nl_msg) {
+virReportOOMError();
+return -1;
+}
+
+if (nlmsg_append(nl_msg, ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) 0)
+goto buffer_too_small;
+
+if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) 0)
+goto buffer_too_small;
+
+if (virNetlinkCommand(nl_msg, resp, recvbuflen, 0, 0,
+ NETLINK_ROUTE, 0) 0) {
+goto cleanup;
+}
+
+if (recvbuflen NLMSG_LENGTH(0) || resp == NULL)
+goto malformed_resp;
+
+switch (resp-nlmsg_type) {
+case NLMSG_ERROR:
+err = (struct nlmsgerr *)NLMSG_DATA(resp);
+if (resp-nlmsg_len NLMSG_LENGTH(sizeof(*err)))
+goto malformed_resp;
+
+if (err-error) {
+virReportSystemError(-err-error,
+ _(error destroying network device %s),
+ ifname);
+goto cleanup;
+}
+break;
+
+case NLMSG_DONE:
+break;
+
+default:
+goto malformed_resp;
+}
+
+rc = 0;
+ cleanup:
+nlmsg_free(nl_msg);
+VIR_FREE(resp);
+return rc;
+
+ malformed_resp:
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(malformed netlink response message));
+goto cleanup;
+
+ buffer_too_small:
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(allocated netlink buffer is too small));
+goto cleanup;
+}
+
+
int
virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen)
{
@@ -803,6 +884,14 @@ int virNetlinkCommand(struct nl_msg *nl_msg
ATTRIBUTE_UNUSED,
return -1;
}
+
+int
+virNetlinkDelLink(const char *ifname ATTRIBUTE_UNSUPPORTED)
+{
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(unsupported));
+return -1;
+}
+
/**
* stopNetlinkEventServer: stop the monitor to receive netlink
* messages for libvirtd
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 1a3e06d..06c3cd0 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2013, 2015 Red Hat, Inc.
* Copyright (C) 2010-2012 IBM Corporation
*
* This library is free software; you can redistribute it and/or
@@ -51,6 +51,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
struct nlmsghdr **resp, unsigned int *respbuflen,
uint32_t src_pid, uint32_t dst_pid,
unsigned int protocol, unsigned int groups);
+int virNetlinkDelLink(const char *ifname);
Like the virNetDevMacVLanDelete which this is essentially replacing -
add a :
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
ACK with the change
John
int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
On 03/23/2015 03:43 PM, Laine Stump wrote: These two functions are identical, so no sense in having the duplication. I resisted the temptation to replace calls to virNetDevMacVLanDelete() with calls to virNetlinkDelLink() just in case some mythical future platform has macvtap devices that aren't managed with netlink (or in case we some day need to do more than just tell the kernel to delete the device). --- src/util/virnetdevmacvlan.c | 67 ++--- 1 file changed, 2 insertions(+), 65 deletions(-) ACK - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] util: use netlink to create bridge devices
On 03/23/2015 03:43 PM, Laine Stump wrote: Just as it is possible to delete a bridge device with the netlink RTM_DELLINK message, one can be created with the RTM_NEWLINK message. Because of differences in the format of the message, it's not as straightforward as with virNetlinkDelLink() to create a single utility function that can be used to create any type of interface, so the new netlink version of virNetDevBridgeCreate() does its own construction of the netlink message and calls virNetlinkCommand() itself. This doesn't provide any extra functionality, just provides symmetry with the previous commit. NB: We *could* alter the API of virNetDevBridgeCreate() to take a MAC address, and directly program that mac address into the bridge (by adding an IFLA_ADDRESS attribute, as is done in virNetDevMacVLanCreate()) rather than separately creating the dummy tap (e.g. virbr0-nic) to maintain a fixed mac address on the bridge, but the commit history of virnetdevbridge.c shows that the presence of this dummy tap is essential in some older versions of the kernel (between 2.6.39 and 3.1 or 3.2, possibly?) to proper operation of IPv6 DAD, and I don't want to take the chance of breaking something that I don't have the time/setup to test (my RHEL6 box is at kernel 2.6.32-544, and the next lowest kernel I have is 3.17) --- src/util/virnetdevbridge.c | 78 +- 1 file changed, 77 insertions(+), 1 deletion(-) What's here seems reasonable - I don't have history on my side to answer the older versions of the kernel query... One other difference between this and virNetDevMacVLanCreate is the error path that handles *retry if the name is already in use... Whether that's a worthy addition here or not is your call... ACK - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
On 03/23/2015 03:43 PM, Laine Stump wrote: These two functions are identical, so no sense in having the duplication. I resisted the temptation to replace calls to virNetDevMacVLanDelete() with calls to virNetlinkDelLink() just in case some mythical future platform has macvtap devices that aren't managed with netlink (or in case we some day need to do more than just tell the kernel to delete the device). --- src/util/virnetdevmacvlan.c | 67 ++--- 1 file changed, 2 insertions(+), 65 deletions(-) hmm interesting... Started reading the next patch and noticed something... Perhaps I was quick on the trigger finger for this one... This module was compiled with if WITH_MACVTAP, but the API being replaced/called uses #if defined(__linux__) defined(HAVE_LIBNL) thus this module never cared about linux HAVE_LIBNL, but now depends on an API that does. My reading comprehension of the Makefile in order to determine whether anything matters is limited... Of course this module has libnl calls in it already so perhaps either WITH_MACVTAP implies __linux__ and HAVE_LIBNL or perhaps that something that needs to be adjusted. Not my specialty! John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] util: use netlink to delete bridge devices
On 03/23/2015 03:43 PM, Laine Stump wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1125755 reported that a stray bridge device was left on the system when a libvirt network failed to start due to an illegal iptables rule caused by bad config. Apparently the reason this was happening was that NetworkManager was noticing immediately when the bridge device was created and automatically setting it IFF_UP. libvirt would then try to setup the iptables rules, get an error back, and since libvirt had never IFF_UPed the bridge, it didn't expect that it needed to set it ~IFF_UP before deleting it during the cleanup process. But the ioctl(SIOCBRDELBR) ioctl will fail to delete a bridge if it is IFF_UP. Since that bug was reported, NetworkManager has gotten a bit more polite in this respect, but just in case something similar happens in the future, this patch switches to using the netlink RTM_DELLINK message to delete the bridge - unlike SIOCBRDELBR, it will delete the requested bridge no matter what the setting of IFF_UP. --- src/util/virnetdevbridge.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) ACK - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemucaps2xmltest: fix the test to correspond to new domain formatting
On Tue, Mar 24, 2015 at 15:31:09 +0100, Pavel Hrdina wrote:
Commit 2360fe5d updated formating of domain element but forget to
s/forget/forgot/
update qemucaps2xmldata xml files. In addition the test code was broken
too. Update the xml files and return -1 if testCompareXMLToXML fails
together with indentation fix.
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
tests/qemucaps2xmldata/all_1.6.0-1.xml| 3 +--
tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 3 +--
tests/qemucaps2xmltest.c | 4 ++--
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/tests/qemucaps2xmldata/all_1.6.0-1.xml
b/tests/qemucaps2xmldata/all_1.6.0-1.xml
index 425b22e..2489f49 100644
--- a/tests/qemucaps2xmldata/all_1.6.0-1.xml
+++ b/tests/qemucaps2xmldata/all_1.6.0-1.xml
@@ -12,8 +12,7 @@
arch name='i686'
wordsize32/wordsize
emulator/usr/bin/qemu-system-i386/emulator
- domain type='qemu'
- /domain
+ domain type='qemu'/
domain type='kvm'
emulator/usr/bin/qemu-system-i386/emulator
/domain
diff --git a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
index cd19814..281fab0 100644
--- a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
+++ b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
@@ -12,8 +12,7 @@
arch name='i686'
wordsize32/wordsize
emulator/usr/bin/qemu-system-i386/emulator
- domain type='qemu'
- /domain
+ domain type='qemu'/
domain type='kvm'
emulator/usr/bin/qemu-system-i386/emulator
/domain
I'd split this patch at this point, as the hunks above fix a test data
failure, while the hunks below fix a bug in the test code.
diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c
index 3529acb..b3975b4 100644
--- a/tests/qemucaps2xmltest.c
+++ b/tests/qemucaps2xmltest.c
@@ -31,7 +31,7 @@
static int
testCompareXMLToXML(const char *inxmldata, const char *outxmldata)
{
-int ret = 1;
+int ret = -1;
if (STRNEQ(outxmldata, inxmldata)) {
virtTestDifference(stderr, outxmldata, inxmldata);
@@ -143,7 +143,7 @@ testQemuCapsXML(const void *opaque)
char *capsXml = NULL;
virCapsPtr capsProvided = NULL;
- if (virAsprintf(xmlFile, %s/qemucaps2xmldata/%s.xml,
+if (virAsprintf(xmlFile, %s/qemucaps2xmldata/%s.xml,
abs_srcdir, data-base) 0)
goto cleanup;
ACK as is. It's not worth splitting/respinning.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] qemu: cleanup setvcpus
On Fri, Mar 20, 2015 at 15:39:01 +0100, Pavel Hrdina wrote: Remove unnecessary maximum variable and also exclusive flags check as they are now tested by upper layer for all drivers. The part about the exclusive flag check is not true. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_driver.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) ACK with commit message fixed. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/2] network_conf: check if bridge exists on host for user created bridges
On 03/24/2015 05:59 AM, Shivaprasad G Bhat wrote:
virNetworkBridgeInUse() doesn't check if the bridge is manually created
outside of libvirt. Check if the bridge actually exist on host using the
virNetDevExists().
Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
---
src/conf/network_conf.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d7c5dec..c3ae2e4 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3227,13 +3227,22 @@ virNetworkBridgeInUseHelper(const void *payload,
int ret = 0;
virNetworkObjPtr net = (virNetworkObjPtr) payload;
const struct virNetworkBridgeInUseHelperData *data = opaque;
+bool defined_bridge = false;
virObjectLock(net);
if (net-def-bridge
-STREQ(net-def-bridge, data-bridge)
-!(data-skipname STREQ(net-def-name, data-skipname)))
-ret = 1;
+STREQ(net-def-bridge, data-bridge)) {
+defined_bridge = true;
+if (!(data-skipname STREQ(net-def-name, data-skipname)))
+ ret = 1;
+}
+
virObjectUnlock(net);
+
+/* Bridge might have been created by a user manually outside libvirt */
+if (!defined_bridge !ret)
+ret = virNetDevExists(data-bridge) ? 1 : 0;
+
return ret;
}
This function is intended to be called once for each defined network on
the host, with data-bridge being the same each time, but
net-def-bridge being different, so adding the check for data-bridge
existence here may work, but it's a bit convoluted.
Instead, you should leave this function alone, and just add the extra
check directly in the function virNetworkBridgeInUse() (either before
locking, or after unlocking nets).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] use new macro helpers to check exclusive flags
On Fri, Mar 20, 2015 at 15:39:00 +0100, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/libvirt-domain-snapshot.c | 45 ++ src/libvirt-domain.c | 288 +++-- src/qemu/qemu_driver.c | 9 +- src/storage/storage_backend_disk.c | 10 +- src/storage/storage_backend_fs.c | 11 +- 5 files changed, 106 insertions(+), 257 deletions(-) I don't object to this change, but as this changes a lot of error messages I'd like to have a second opinion on this. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] conf: Add support for storage state directory
On Tue, Mar 24, 2015 at 10:16:25 -0400, John Ferlan wrote: On 03/24/2015 06:06 AM, Erik Skultety wrote: Before introducing necessary changes to storage_driver.c, first prepare our structures for storage state XML support. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733 --- src/conf/storage_conf.h | 1 + src/storage/storage_driver.c | 1 + 2 files changed, 2 insertions(+) I had started down this path before for a different bug/issue: http://www.redhat.com/archives/libvir-list/2015-January/msg00465.html but abandoned the stateDir because it was felt it wasn't necessary. I recall running into a few interesting issues with stateDir including fixing one issue seen during testing that didn't hit the list. Good The patches above are for status XMLs for storage volumes while this series does the same for storage pools. While volumes can theoretically be reloaded from existing pools this series fixes storage pools that vanish during start of libvirt. news is I still have the patches in a branch somewhere if you're interested. 1 2 are on list... The 3rd one in the archive was a solution to the particular problem - that was rejected and a different solution was pushed. In any case, I do suggest looking at 1 2, plus I can send you an adjustment to 1 to resolve some condition I ran into, but cannot recall the details. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: change accidental VIR_WARNING back to VIR_DEBUG
While debugging the support for responding to qemu RX_FILTER_CHANGED
events, I had changed the ignoring this event log message from
VIR_DEBUG to VIR_WARN, but forgot to change it back before
pushing. Since many guest OSes make enough changes to multicast lists
and/or promiscuous mode settings to trigger this message, it's
starting to show up as a red herring in bug reports.
---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6d9217b..2bac4a9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4340,7 +4340,7 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
def = dev.data.net;
if (!virDomainNetGetActualTrustGuestRxFilters(def)) {
-VIR_WARN(ignore NIC_RX_FILTER_CHANGED event for network
+VIR_DEBUG(ignore NIC_RX_FILTER_CHANGED event for network
device %s in domain %s,
def-info.alias, vm-def-name);
/* not sending query-rx-filter will also suppress any
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemucaps2xmltest: fix the test to correspond to new domain formatting
On Tue, Mar 24, 2015 at 04:36:09PM +0100, Peter Krempa wrote:
On Tue, Mar 24, 2015 at 15:31:09 +0100, Pavel Hrdina wrote:
Commit 2360fe5d updated formating of domain element but forget to
s/forget/forgot/
update qemucaps2xmldata xml files. In addition the test code was broken
too. Update the xml files and return -1 if testCompareXMLToXML fails
together with indentation fix.
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
tests/qemucaps2xmldata/all_1.6.0-1.xml| 3 +--
tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 3 +--
tests/qemucaps2xmltest.c | 4 ++--
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/tests/qemucaps2xmldata/all_1.6.0-1.xml
b/tests/qemucaps2xmldata/all_1.6.0-1.xml
index 425b22e..2489f49 100644
--- a/tests/qemucaps2xmldata/all_1.6.0-1.xml
+++ b/tests/qemucaps2xmldata/all_1.6.0-1.xml
@@ -12,8 +12,7 @@
arch name='i686'
wordsize32/wordsize
emulator/usr/bin/qemu-system-i386/emulator
- domain type='qemu'
- /domain
+ domain type='qemu'/
domain type='kvm'
emulator/usr/bin/qemu-system-i386/emulator
/domain
diff --git a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
index cd19814..281fab0 100644
--- a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
+++ b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
@@ -12,8 +12,7 @@
arch name='i686'
wordsize32/wordsize
emulator/usr/bin/qemu-system-i386/emulator
- domain type='qemu'
- /domain
+ domain type='qemu'/
domain type='kvm'
emulator/usr/bin/qemu-system-i386/emulator
/domain
I'd split this patch at this point, as the hunks above fix a test data
failure, while the hunks below fix a bug in the test code.
diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c
index 3529acb..b3975b4 100644
--- a/tests/qemucaps2xmltest.c
+++ b/tests/qemucaps2xmltest.c
@@ -31,7 +31,7 @@
static int
testCompareXMLToXML(const char *inxmldata, const char *outxmldata)
{
-int ret = 1;
+int ret = -1;
if (STRNEQ(outxmldata, inxmldata)) {
virtTestDifference(stderr, outxmldata, inxmldata);
@@ -143,7 +143,7 @@ testQemuCapsXML(const void *opaque)
char *capsXml = NULL;
virCapsPtr capsProvided = NULL;
- if (virAsprintf(xmlFile, %s/qemucaps2xmldata/%s.xml,
+if (virAsprintf(xmlFile, %s/qemucaps2xmldata/%s.xml,
abs_srcdir, data-base) 0)
goto cleanup;
ACK as is. It's not worth splitting/respinning.
Peter
Thanks, pushed now.
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] internal: introduce macro helpers to reject exclusive flags
On Fri, Mar 20, 2015 at 15:38:59 +0100, Pavel Hrdina wrote:
Inspired by commit 7e437ee7 that introduced similar macros for virsh
commands so we don't have to repeat the same code all over.
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/internal.h | 80
++
1 file changed, 80 insertions(+)
diff --git a/src/internal.h b/src/internal.h
index 4d473af..5885f03 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -327,6 +327,86 @@
} \
} while (0)
+/* Macros to help dealing with mutually exclusive flags. */
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_EXPR_RET:
+ *
+ * @NAME1: String containing the name of the flag.
+ * @EXPR1: Expression to validate the flag.
+ * @NAME2: String containing the name of the flag.
+ * @EXPR2: Expression to validate the flag.
+ *
+ * Reject mutually exclusive API flags. Use the provided expression
+ * to check the flags.
+ *
+ * This helper does an early return and therefore it has to be called
+ * before anything that would require cleanup.
+ */
+# define VIR_EXCLUSIVE_FLAGS_EXPR_RET(NAME1, EXPR1, NAME2, EXPR2) \
+if ((EXPR1) (EXPR2)) { \
+virReportInvalidArg(ctl,\
+_(Flags '%s' and '%s' are mutually exclusive),\
+NAME1, NAME2); \
+return -1; \
+}
+
While in virsh the above variant makes sense, because in some cases the
variables have different names than the corresponding flags, in case of
this code having it doesn't make sense.
+/**
+ * VIR_EXCLUSIVE_FLAGS_VAR_RET:
+ *
+ * @FLAG1: First flag to be checked
+ * @FLAG2: Second flag to be checked
A third argument @RET should be added here so that this can be used in
places that also return NULL or any other possible variable.
In virsh the assumption is that the flags are checked at first and thus
returning false makes sense, as every virsh command does that.
+ *
+ * Reject mutually exclusive API flags. The checked flags are compared
+ * with flags variable.
+ *
+ * This helper does an early return and therefore it has to be called
+ * before anything that would require cleanup.
+ */
+# define VIR_EXCLUSIVE_FLAGS_RET(FLAG1, FLAG2) \
+VIR_EXCLUSIVE_FLAGS_EXPR_RET(#FLAG1, flags FLAG1, #FLAG2, flags
FLAG2)
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_EXPR_GOTO:
+ *
+ * @NAME1: String containing the name of the flag.
+ * @EXPR1: Expression to validate the flag.
+ * @NAME2: String containing the name of the flag.
+ * @EXPR2: Expression to validate the flag.
+ * @LABEL: label to jump to.
+ *
+ * Reject mutually exclusive API flags. Use the provided expression
+ * to check the flag.
+ *
+ * Returns nothing. Jumps to a label if unsupported flags were
+ * passed to it.
+ */
+# define VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(NAME1, EXPR1, NAME2, EXPR2, LABEL) \
+if ((EXPR1) (EXPR2)) { \
+virReportInvalidArg(ctl,\
+_(Flags '%s' and '%s' are mutually exclusive),\
+NAME1, NAME2); \
+goto LABEL; \
+}
+
Again, this function does not make sense in the library code.
+/**
+ * VIR_EXCLUSIVE_FLAGS_VAR_GOTO:
+ *
+ * @FLAG1: First flag to be checked.
+ * @FLAG2: Second flag to be checked.
+ * @LABEL: label to jump to.
+ *
+ * Reject mutually exclusive API flags. The checked flags are compared
+ * with flags variable.
+ *
+ * Returns nothing. Jumps to a label if unsupported flags were
+ * passed to it.
+ */
+# define VIR_EXCLUSIVE_FLAGS_GOTO(FLAG1, FLAG2, LABEL) \
+VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(#FLAG1, flags FLAG1,\
+ #FLAG2, flags FLAG2, LABEL)
+
+
# define virCheckNonNullArgReturn(argname, retval) \
do {\
if (argname == NULL) { \
While this is okay.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemucaps2xmltest: fix the test to correspond to new domain formatting
Commit 2360fe5d updated formating of domain element but forget to
update qemucaps2xmldata xml files. In addition the test code was broken
too. Update the xml files and return -1 if testCompareXMLToXML fails
together with indentation fix.
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
tests/qemucaps2xmldata/all_1.6.0-1.xml| 3 +--
tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 3 +--
tests/qemucaps2xmltest.c | 4 ++--
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/tests/qemucaps2xmldata/all_1.6.0-1.xml
b/tests/qemucaps2xmldata/all_1.6.0-1.xml
index 425b22e..2489f49 100644
--- a/tests/qemucaps2xmldata/all_1.6.0-1.xml
+++ b/tests/qemucaps2xmldata/all_1.6.0-1.xml
@@ -12,8 +12,7 @@
arch name='i686'
wordsize32/wordsize
emulator/usr/bin/qemu-system-i386/emulator
- domain type='qemu'
- /domain
+ domain type='qemu'/
domain type='kvm'
emulator/usr/bin/qemu-system-i386/emulator
/domain
diff --git a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
index cd19814..281fab0 100644
--- a/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
+++ b/tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
@@ -12,8 +12,7 @@
arch name='i686'
wordsize32/wordsize
emulator/usr/bin/qemu-system-i386/emulator
- domain type='qemu'
- /domain
+ domain type='qemu'/
domain type='kvm'
emulator/usr/bin/qemu-system-i386/emulator
/domain
diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c
index 3529acb..b3975b4 100644
--- a/tests/qemucaps2xmltest.c
+++ b/tests/qemucaps2xmltest.c
@@ -31,7 +31,7 @@
static int
testCompareXMLToXML(const char *inxmldata, const char *outxmldata)
{
-int ret = 1;
+int ret = -1;
if (STRNEQ(outxmldata, inxmldata)) {
virtTestDifference(stderr, outxmldata, inxmldata);
@@ -143,7 +143,7 @@ testQemuCapsXML(const void *opaque)
char *capsXml = NULL;
virCapsPtr capsProvided = NULL;
- if (virAsprintf(xmlFile, %s/qemucaps2xmldata/%s.xml,
+if (virAsprintf(xmlFile, %s/qemucaps2xmldata/%s.xml,
abs_srcdir, data-base) 0)
goto cleanup;
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: change accidental VIR_WARNING back to VIR_DEBUG
On 03/24/2015 10:49 AM, Laine Stump wrote: While debugging the support for responding to qemu RX_FILTER_CHANGED events, I had changed the ignoring this event log message from VIR_DEBUG to VIR_WARN, but forgot to change it back before pushing. Since many guest OSes make enough changes to multicast lists and/or promiscuous mode settings to trigger this message, it's starting to show up as a red herring in bug reports. --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK - Ironically I had just started noticing this in a debug session and began wondering... John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: command: Report error when formatting network source with protocol _NONE
The function that formats the string for network drives would return error code but did not set the error message when called on storage source with VIR_STORAGE_NET_PROTOCOL_LAST or _NONE. Report an error in this case if it would ever be called in that way. --- src/qemu/qemu_command.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7e6b95c..8dd7a76 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3263,6 +3263,9 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Unexpected network protocol '%s'), + virStorageNetProtocolTypeToString(src-protocol)); goto cleanup; } -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] qemu: command: Check for empty network source when formatting drive cmd
Use the virStorageSourceIsEmpty helper to determine whether the drive
source is empty rather than checking for src-path. This will fix start
of VM with empty network cdrom that would not report any error.
---
src/qemu/qemu_command.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8dd7a76..43eecf8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3289,6 +3289,10 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
*source = NULL;
+/* return 1 for empty sources */
+if (virStorageSourceIsEmpty(src))
+return 1;
+
if (conn) {
if (actualType == VIR_STORAGE_TYPE_NETWORK
src-auth
@@ -3318,11 +3322,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
case VIR_STORAGE_TYPE_BLOCK:
case VIR_STORAGE_TYPE_FILE:
case VIR_STORAGE_TYPE_DIR:
-if (!src-path) {
-ret = 1;
-goto cleanup;
-}
-
if (VIR_STRDUP(*source, src-path) 0)
goto cleanup;
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Fix unknown error when starting qemu VM with empty network cdrom
Peter Krempa (2): qemu: command: Report error when formatting network source with protocol _NONE qemu: command: Check for empty network source when formatting drive cmd src/qemu/qemu_command.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] docs: route element must specify network address
because network address is required by route, so here we should add one avoid user misunderstand. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3b3d2d9..d7fe942 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4472,7 +4472,7 @@ qemu-kvm -net nic,model=? /dev/null lt;target dev='vnet0'/gt; blt;ip address='192.168.122.5' prefix='24'/gt;/b blt;route family='ipv4' address='192.168.122.0' prefix='24' gateway='192.168.122.1'/gt;/b - blt;route family='ipv4' gateway='192.168.122.1'/gt;/b + blt;route family='ipv4' address='192.168.122.8' gateway='192.168.122.1'/gt;/b lt;/interfacegt; ... lt;hostdev mode='capabilities' type='net'gt; @@ -4481,7 +4481,7 @@ qemu-kvm -net nic,model=? /dev/null lt;/sourcegt; blt;ip address='192.168.122.6' prefix='24'/gt;/b blt;route family='ipv4' address='192.168.122.0' prefix='24' gateway='192.168.122.1'/gt;/b - blt;route family='ipv4' gateway='192.168.122.1'/gt;/b + blt;route family='ipv4' address='192.168.122.8' gateway='192.168.122.1'/gt;/b lt;/hostdevgt; lt;/devicesgt; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] docs: no 'via' attribute in route element
via - gateway Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- docs/formatdomain.html.in | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 82aa14f..3b3d2d9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4471,8 +4471,8 @@ qemu-kvm -net nic,model=? /dev/null lt;source network='default'/gt; lt;target dev='vnet0'/gt; blt;ip address='192.168.122.5' prefix='24'/gt;/b - blt;route family='ipv4' address='192.168.122.0' prefix='24' via='192.168.122.1'/gt;/b - blt;route family='ipv4' via='192.168.122.1'/gt;/b + blt;route family='ipv4' address='192.168.122.0' prefix='24' gateway='192.168.122.1'/gt;/b + blt;route family='ipv4' gateway='192.168.122.1'/gt;/b lt;/interfacegt; ... lt;hostdev mode='capabilities' type='net'gt; @@ -4480,8 +4480,8 @@ qemu-kvm -net nic,model=? /dev/null lt;interfacegt;eth0lt;/interfacegt; lt;/sourcegt; blt;ip address='192.168.122.6' prefix='24'/gt;/b - blt;route family='ipv4' address='192.168.122.0' prefix='24' via='192.168.122.1'/gt;/b - blt;route family='ipv4' via='192.168.122.1'/gt;/b + blt;route family='ipv4' address='192.168.122.0' prefix='24' gateway='192.168.122.1'/gt;/b + blt;route family='ipv4' gateway='192.168.122.1'/gt;/b lt;/hostdevgt; lt;/devicesgt; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] fix domain documentation error
Chen Fan (2): docs: no 'via' attribute in route element docs: route element must specify network address docs/formatdomain.html.in | 8 1 file changed, 4 insertions(+), 4 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 00/12] qemu: add support to hot-plug/unplug cpu device
On 03/24/2015 04:30 PM, Zhi Yong Wu wrote: HI, Do you have plan to update this patchset based on the comments recently or have the latest one to post? The develop plan for cpu hotplug in qemu has been changed, it is to add socket-based device_add. So I think we could not continue this cpu hotplug work in libvirt until the interface about this feature in qemu could be determined. Thanks, Zhu I noticed that the patchset for memory hotplug had got merged, is there any plan to merge this patchset? On Fri, Feb 13, 2015 at 12:13 AM, Peter Krempa pkre...@redhat.com wrote: On Thu, Feb 12, 2015 at 19:53:13 +0800, Zhu Guihua wrote: Hi all, Any comments about this series? I'm not sure whether this series' method to support cpu hotplug in libvirt is reasonable, so could anyone give more suggestions about this function? Thanks. Well, as Dan pointed out in his review for this series and previous version, we are not convinced that we need a way to specify the CPU model and other parameters as having dissimilar CPUs doesn't make sense. A solution is either to build the cpu plug on top of the existing API or provide enough information to convince us that having the cpu model in the XML actually makes sense. Regards, Zhu Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] qemu: check if domain is really active when do setvcpus with --live
On Fri, Mar 20, 2015 at 15:39:02 +0100, Pavel Hrdina wrote: From: Luyao Huang lhu...@redhat.com Commit e105dc9 fix setting vcpus for offline domain, but forget check if vm is active when pass VIR_DOMAIN_AFFECT_LIVE flags. # virsh setvcpus test3 4 --live error: Failed to create controller cpu for group: No such file or directory Add a check if we pass VIR_DOMAIN_AFFECT_LIVE flags. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204006 Signed-off-by: Luyao Huang lhu...@redhat.com Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_driver.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) NACK, after the virDomainLiveConfigHelperMethod gets moved, this is not logner necessary as it contains the same code. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 6/6] Auto add virtio-serial controllers
In virDomainVirtioSerialAddrNext, add another controller
if we've exhausted all ports of the existing controllers.
https://bugzilla.redhat.com/show_bug.cgi?id=1076708
---
src/conf/domain_addr.c | 47 +++---
src/conf/domain_addr.h | 10 +++--
src/qemu/qemu_command.c| 4 +-
src/qemu/qemu_hotplug.c| 3 +-
.../qemuxml2argv-channel-virtio-autoadd.args | 33 +++
.../qemuxml2argv-channel-virtio-autoadd.xml| 45 +
tests/qemuxml2argvtest.c | 2 +
7 files changed, 132 insertions(+), 12 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoadd.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoadd.xml
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 49d28be..27a237d 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -878,7 +878,28 @@
virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs)
}
static int
-virDomainVirtioSerialAddrNext(virDomainVirtioSerialAddrSetPtr addrs,
+virDomainVirtioSerialAddrSetAutoaddController(virDomainDefPtr def,
+ virDomainVirtioSerialAddrSetPtr
addrs,
+ unsigned int idx)
+{
+int contidx;
+
+if (virDomainDefMaybeAddController(def,
+
VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL,
+ idx, -1) 0)
+return -1;
+
+contidx = virDomainControllerFind(def,
VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, idx);
+
+if (virDomainVirtioSerialAddrSetAddController(addrs,
def-controllers[contidx]) 0)
+return -1;
+
+return 0;
+}
+
+static int
+virDomainVirtioSerialAddrNext(virDomainDefPtr def,
+ virDomainVirtioSerialAddrSetPtr addrs,
virDomainDeviceVirtioSerialAddress *addr,
bool allowZero)
{
@@ -905,6 +926,20 @@
virDomainVirtioSerialAddrNext(virDomainVirtioSerialAddrSetPtr addrs,
}
}
+if (def) {
+for (i = 0; i INT_MAX; i++) {
+int idx = virDomainControllerFind(def,
VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, i);
+
+if (idx == -1) {
+if (virDomainVirtioSerialAddrSetAutoaddController(def, addrs,
i) 0)
+goto cleanup;
+controller = i;
+port = startPort + 1;
+goto success;
+}
+}
+}
+
virReportError(VIR_ERR_XML_ERROR, %s,
_(Unable to find a free virtio-serial port));
@@ -958,7 +993,8 @@
virDomainVirtioSerialAddrNextFromController(virDomainVirtioSerialAddrSetPtr addr
* or assign a virtio serial address to the device
*/
int
-virDomainVirtioSerialAddrAutoAssign(virDomainVirtioSerialAddrSetPtr addrs,
+virDomainVirtioSerialAddrAutoAssign(virDomainDefPtr def,
+virDomainVirtioSerialAddrSetPtr addrs,
virDomainDeviceInfoPtr info,
bool allowZero)
{
@@ -967,12 +1003,13 @@
virDomainVirtioSerialAddrAutoAssign(virDomainVirtioSerialAddrSetPtr addrs,
info-addr.vioserial.port)
return virDomainVirtioSerialAddrReserve(NULL, NULL, info, addrs);
else
-return virDomainVirtioSerialAddrAssign(addrs, info, allowZero,
portOnly);
+return virDomainVirtioSerialAddrAssign(def, addrs, info, allowZero,
portOnly);
}
int
-virDomainVirtioSerialAddrAssign(virDomainVirtioSerialAddrSetPtr addrs,
+virDomainVirtioSerialAddrAssign(virDomainDefPtr def,
+virDomainVirtioSerialAddrSetPtr addrs,
virDomainDeviceInfoPtr info,
bool allowZero,
bool portOnly)
@@ -988,7 +1025,7 @@
virDomainVirtioSerialAddrAssign(virDomainVirtioSerialAddrSetPtr addrs,
ptr-addr.vioserial)
0)
goto cleanup;
} else {
-if (virDomainVirtioSerialAddrNext(addrs, ptr-addr.vioserial,
+if (virDomainVirtioSerialAddrNext(def, addrs, ptr-addr.vioserial,
allowZero) 0)
goto cleanup;
}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 14ffd17..c18e130 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -208,17 +208,19 @@
virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs);
bool
virDomainVirtioSerialAddrIsComplete(virDomainDeviceInfoPtr info);
int
-virDomainVirtioSerialAddrAutoAssign(virDomainVirtioSerialAddrSetPtr addrs,
+virDomainVirtioSerialAddrAutoAssign(virDomainDefPtr def,
+
[libvirt] [PATCHv3 5/6] Assign an address when hotplugging a virtio-serial device
---
src/qemu/qemu_hotplug.c | 21 +++--
tests/qemuhotplugtest.c | 2 +-
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ef2f9b0..560a164 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1541,6 +1541,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
virDomainDefPtr vmdef = vm-def;
char *devstr = NULL;
char *charAlias = NULL;
+bool need_release = false;
+bool allowZero = false;
if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
@@ -1551,6 +1553,16 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (qemuAssignDeviceChrAlias(vmdef, chr, -1) 0)
goto cleanup;
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE
+chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)
+allowZero = true;
+
+if (virDomainVirtioSerialAddrAutoAssign(priv-vioserialaddrs,
+chr-info,
+allowZero) 0)
+goto cleanup;
+need_release = true;
+
if (qemuBuildChrDeviceStr(devstr, vm-def, chr, priv-qemuCaps) 0)
goto cleanup;
@@ -1582,6 +1594,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
cleanup:
if (ret 0 virDomainObjIsActive(vm))
qemuDomainChrInsertPreAllocCleanup(vm-def, chr);
+if (ret 0 need_release)
+virDomainVirtioSerialAddrRelease(priv-vioserialaddrs, chr-info);
VIR_FREE(charAlias);
VIR_FREE(devstr);
return ret;
@@ -4120,10 +4134,13 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
goto cleanup;
rc = qemuDomainWaitForDeviceRemoval(vm);
-if (rc == 0 || rc == 1)
+if (rc == 0 || rc == 1) {
+virDomainVirtioSerialAddrRelease(priv-vioserialaddrs, tmpChr-info);
ret = qemuDomainRemoveChrDevice(driver, vm, tmpChr);
-else
+} else {
ret = 0;
+}
+
cleanup:
qemuDomainResetDeviceRemoval(vm);
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 12a7f71..ea2cf77 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -86,7 +86,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
if (event)
virQEMUCapsSet(priv-qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT);
-if (qemuDomainAssignPCIAddresses((*vm)-def, priv-qemuCaps, *vm) 0)
+if (qemuDomainAssignAddresses((*vm)-def, priv-qemuCaps, *vm) 0)
goto cleanup;
if (qemuAssignDeviceAliases((*vm)-def, priv-qemuCaps) 0)
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 1/6] Add test for virtio serial port assignment
Add a test to demonstrate the effect of automatic virtio-serial address assignment. --- .../qemuxml2argv-channel-virtio-autoassign.args| 20 + .../qemuxml2argv-channel-virtio-autoassign.xml | 50 ++ tests/qemuxml2argvtest.c | 2 + 3 files changed, 72 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args new file mode 100644 index 000..f7f7b8d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args @@ -0,0 +1,20 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ +-device virtio-serial-pci,id=virtio-serial0,max_ports=4,vectors=4,bus=pci.0\ +,addr=0x3 -device virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa \ +-usb -hda /dev/HostVG/QEMUGuest1 \ +-chardev pty,id=charchannel0 -device virtserialport,bus=virtio-serial0.0,nr=1,\ +chardev=charchannel0,id=channel0,name=org.linux-kvm.port.0 \ +-chardev pty,id=charchannel1 -device virtserialport,bus=virtio-serial0.2,nr=1,\ +chardev=charchannel1,id=channel1,name=org.linux-kvm.port.foo \ +-chardev pty,id=charchannel2 -device virtserialport,bus=virtio-serial0.0,nr=1,\ +chardev=charchannel2,id=channel2,name=org.linux-kvm.port.bar \ +-chardev pty,id=charchannel3 -device virtserialport,bus=virtio-serial0.0,nr=2,\ +chardev=charchannel3,id=channel3,name=org.linux-kvm.port.wizz \ +-chardev pty,id=charchannel4 -device virtserialport,bus=virtio-serial0.0,nr=3,\ +chardev=charchannel4,id=channel4,name=org.linux-kvm.port.ooh \ +-chardev pty,id=charchannel5 -device virtserialport,bus=virtio-serial0.0,nr=4,\ +chardev=charchannel5,id=channel5,name=org.linux-kvm.port.lla \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml new file mode 100644 index 000..5592c8f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml @@ -0,0 +1,50 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static' cpuset='1-4,8-20,525'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='virtio-serial' index='0' ports='4' vectors='4'/ +controller type='virtio-serial' index='1' + address type='pci' domain='0x' bus='0x00' slot='0x0a' function='0x0'/ +/controller +channel type='pty' + target type='virtio' name='org.linux-kvm.port.0'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.foo'/ + address type='virtio-serial' controller='0' bus='2'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.bar'/ + address type='virtio-serial' controller='0' port='1'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.wizz'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.ooh'/ +/channel +channel type='pty' + target type='virtio' name='org.linux-kvm.port.lla'/ +/channel +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 08f374e..00e608c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1070,6 +1070,8 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST(channel-virtio-auto, QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); +DO_TEST(channel-virtio-autoassign, +QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST(console-virtio, QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST(console-virtio-many, -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/2] cleanup conf/device_conf.h inclusion from util/virnetdev.h
On 03/24/2015 05:57 AM, Shivaprasad G Bhat wrote:
Move the struct and enum definitions from device_conf.h to
virnetdev.h thus avoiding the file inclusion.
The wrong reference of conf files from util/ is allowed
in Makefile.am for now. The reason being,
The change in Makefile.am for libvirt_util_la_CFLAGS to remove
conf from the utils would require corresponding inclusion in util files
to specify the paths explicitly. This would result in syntax-check failures
(prohibit_cross_inclusion) which need to be worked around until
there are patches reworking the incorrect inclusion.
I agree that we should eliminate this #include of something from conf in
util (and definitely that is a more egregious problem than calling a
platform specific function from conf), but disagree with using this
patch as the method of assuring that virNetDevExists() is declared for
network_conf.c (so that patch 2/2 of this series compiles).
if virnetdev.h is required by something in network_conf.c, then it
should be #included by network_conf.c. (yes, I understand that the code
movement in this patch makes it necessary to #include virnetdev.h in
device_conf.h anyway)
I see that there are a few other cases of *_conf.h files being included
from .h files in the util directory. Is this patch perhaps part of a
patch series to fix that?
(BTW, the util directory was clean of any #includes from the conf
directory until last July, when management of close callbacks was
moved from the qemu driver into util/virclosecallbacks.h. The problem is
that one of the args to the a callback is a virDomainObj. So I think the
proper solution to this problem would require making that arg to all the
virCloseCallback functions an opaque pointer, show real nature would be
known only to the callback function itself.)
The syntax-check failure workaround seems dangerous as that might mask some
easily resolvable failures. So dont touch the Makefile.am for now. Resolve
the wrong inclusions in separate patches.
Maybe that's what you're referring to here?
Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
---
src/conf/device_conf.h | 38 +-
src/util/virnetdev.h | 38 +-
2 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index 7ea90f6..a650189 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -31,19 +31,7 @@
# include virutil.h
# include virthread.h
# include virbuffer.h
-
-typedef enum {
-VIR_INTERFACE_STATE_UNKNOWN = 1,
-VIR_INTERFACE_STATE_NOT_PRESENT,
-VIR_INTERFACE_STATE_DOWN,
-VIR_INTERFACE_STATE_LOWER_LAYER_DOWN,
-VIR_INTERFACE_STATE_TESTING,
-VIR_INTERFACE_STATE_DORMANT,
-VIR_INTERFACE_STATE_UP,
-VIR_INTERFACE_STATE_LAST
-} virInterfaceState;
-
-VIR_ENUM_DECL(virInterfaceState)
+# include virnetdev.h
typedef struct _virDevicePCIAddress virDevicePCIAddress;
typedef virDevicePCIAddress *virDevicePCIAddressPtr;
@@ -55,30 +43,6 @@ struct _virDevicePCIAddress {
int multi; /* virTristateSwitch */
};
-typedef struct _virInterfaceLink virInterfaceLink;
-typedef virInterfaceLink *virInterfaceLinkPtr;
-struct _virInterfaceLink {
-virInterfaceState state; /* link state */
-unsigned int speed; /* link speed in Mbits per second */
-};
-
-typedef enum {
-VIR_NET_DEV_FEAT_GRXCSUM,
-VIR_NET_DEV_FEAT_GTXCSUM,
-VIR_NET_DEV_FEAT_GSG,
-VIR_NET_DEV_FEAT_GTSO,
-VIR_NET_DEV_FEAT_GGSO,
-VIR_NET_DEV_FEAT_GGRO,
-VIR_NET_DEV_FEAT_LRO,
-VIR_NET_DEV_FEAT_RXVLAN,
-VIR_NET_DEV_FEAT_TXVLAN,
-VIR_NET_DEV_FEAT_NTUPLE,
-VIR_NET_DEV_FEAT_RXHASH,
-VIR_NET_DEV_FEAT_LAST
-} virNetDevFeature;
-
-VIR_ENUM_DECL(virNetDevFeature)
-
int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr);
int virDevicePCIAddressParseXML(xmlNodePtr node,
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 856127b..daba0bb 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -30,7 +30,6 @@
# include virnetlink.h
# include virmacaddr.h
# include virpci.h
-# include device_conf.h
# ifdef HAVE_STRUCT_IFREQ
typedef struct ifreq virIfreq;
@@ -47,6 +46,43 @@ typedef enum {
} virNetDevRxFilterMode;
VIR_ENUM_DECL(virNetDevRxFilterMode)
+typedef enum {
+VIR_INTERFACE_STATE_UNKNOWN = 1,
+VIR_INTERFACE_STATE_NOT_PRESENT,
+VIR_INTERFACE_STATE_DOWN,
+VIR_INTERFACE_STATE_LOWER_LAYER_DOWN,
+VIR_INTERFACE_STATE_TESTING,
+VIR_INTERFACE_STATE_DORMANT,
+VIR_INTERFACE_STATE_UP,
+VIR_INTERFACE_STATE_LAST
+} virInterfaceState;
+
+VIR_ENUM_DECL(virInterfaceState)
+
+typedef struct _virInterfaceLink virInterfaceLink;
+typedef virInterfaceLink *virInterfaceLinkPtr;
+struct _virInterfaceLink {
+virInterfaceState state; /* link state */
+unsigned int speed; /* link speed in
Re: [libvirt] [Xen-devel] [PATCH] libxl: fix dom0 balloon logic
On Mon, 23 Mar 2015, Ian Campbell wrote:
(just ccing the other tools maintainers, in particular Stefano who knows
what this stuff is supposed to do...)
On Fri, 2015-03-20 at 17:10 -0600, Jim Fehlig wrote:
Recent testing on large memory systems revealed a bug in the Xen xl
tool's freemem() function. When autoballooning is enabled, freemem()
is used to ensure enough memory is available to start a domain,
ballooning dom0 if necessary. When ballooning large amounts of memory
from dom0, freemem() would exceed its self-imposed wait time and
return an error. Meanwhile, dom0 continued to balloon. Starting the
domain later, after sufficient memory was ballooned from dom0, would
succeed. The libvirt implementation in libxlDomainFreeMem() suffers
the same bug since it is modeled after freemem().
In the end, the best place to fix the bug on the Xen side was to
slightly change the behavior of libxl_wait_for_memory_target().
Instead of failing after caller-provided wait_sec, the function now
blocks as long as dom0 memory ballooning is progressing. It will return
failure only when more memory is needed to reach the target and wait_sec
have expired with no progress being made. See xen.git commit fd3aa246.
There was a dicussion on how this would affect other libxl apps like
libvirt
http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html
If libvirt containing this patch was build against a Xen containing
the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem()
will fail after 30 sec and domain creation will be terminated.
Without this patch and with old libxl_wait_for_memory_target() behavior,
libxlDomainFreeMem() does not succeed after 30 sec, but returns success
anyway. Domain creation continues resulting in all sorts of fun stuff
like cpu soft lockups in the guest OS. It was decided to properly fix
libxl_wait_for_memory_target(), and if anything improve the default
behavior of apps using the freemem reference impl in xl.
xl was patched to accommodate the change in libxl_wait_for_memory_target()
with xen.git commit 883b30a0. This patch does the same in the libxl
driver. While at it, I changed the logic to essentially match
freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner
IMO and will make it easier to spot future, potentially interesting
divergences.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/libxl/libxl_domain.c | 57
1 file changed, 28 insertions(+), 29 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 407a9bd..ff78133 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1121,38 +1121,41 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv,
libxl_domain_config *d_config)
{
uint32_t needed_mem;
uint32_t free_mem;
-size_t i;
-int ret = -1;
+int ret;
int tries = 3;
int wait_secs = 10;
-if ((ret = libxl_domain_need_memory(priv-ctx, d_config-b_info,
-needed_mem)) = 0) {
-for (i = 0; i tries; ++i) {
-if ((ret = libxl_get_free_memory(priv-ctx, free_mem)) 0)
-break;
+ret = libxl_domain_need_memory(priv-ctx, d_config-b_info,
+ needed_mem);
+if (ret 0)
+goto error;
-if (free_mem = needed_mem) {
-ret = 0;
-break;
-}
+do {
+ret = libxl_get_free_memory(priv-ctx, free_mem);
+if (ret 0)
+goto error;
-if ((ret = libxl_set_memory_target(priv-ctx, 0,
- free_mem - needed_mem,
- /* relative */ 1, 0)) 0)
-break;
+if (free_mem = needed_mem)
+return 0;
-ret = libxl_wait_for_free_memory(priv-ctx, 0, needed_mem,
- wait_secs);
-if (ret == 0 || ret != ERROR_NOMEM)
-break;
+ret = libxl_set_memory_target(priv-ctx, 0,
+ free_mem - needed_mem,
+ /* relative */ 1, 0);
+if (ret 0)
+goto error;
-if ((ret = libxl_wait_for_memory_target(priv-ctx, 0, 1)) 0)
-break;
-}
-}
+ret = libxl_wait_for_free_memory(priv-ctx, 0, needed_mem,
+ wait_secs);
+if (ret 0)
+goto error;
Shouldn't this be a call to libxl_wait_for_memory_target instead?
-return ret;
+tries--;
+} while (tries 0);
+
+ error:
+virReportSystemError(ret, %s,
+ _(Failed to
Re: [libvirt] [PATCH 7/6] qemu: move virDomainLiveConfigHelperMethod right after BeginJob
On Tue, Mar 24, 2015 at 05:13:07PM +0100, Peter Krempa wrote: On Fri, Mar 20, 2015 at 15:58:56 +0100, Pavel Hrdina wrote: We should call virDomainLiveConfigHelperMethod ASAP because this function transfers VIR_DOMAIN_AFFECT_CURRENT to VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. All other additional checks for those two flags should consider that the user give us VIR_DOMAIN_AFFECT_CURRENT. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_driver.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) ACK, although this should be 0.5/6 or 1.5/6. Basically just push this first. Peter Thanks for review, I'll push it shortly. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] tools: fix the wrong check when use virsh setvcpus --maximum
On Tue, Mar 24, 2015 at 05:34:31PM +0100, Peter Krempa wrote: On Fri, Mar 20, 2015 at 15:39:03 +0100, Pavel Hrdina wrote: From: Luyao Huang lhu...@redhat.com We will ignore --maximum option when only use setvcpus with this option, like this (this error is another issue): # virsh setvcpus test3 --maximum 10 error: Failed to create controller cpu for group: No such file or directory this is because we do not set it in flags before we check if there is a flags set. Refactor these code and fix the logic. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204033 Signed-off-by: Luyao Huang lhu...@redhat.com Signed-off-by: Pavel Hrdina phrd...@redhat.com --- tools/virsh-domain.c | 30 ++ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1d8225c..9430ad9 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6735,6 +6735,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); VSH_EXCLUSIVE_OPTIONS_VAR(guest, config); +VSH_EXCLUSIVE_OPTIONS_VAR(guest, maximum); +VSH_EXCLUSIVE_OPTIONS_VAR(config, maximum); As Yanbing pointed out, you want to make live and maximum exclusive. Additionally this changes semantics, as currently --maximum was possible if and only if --config was specified, which would make it exclusive with --current too. This is also implied in the man page. We have the following options: 1) Make --maximum imply --config and document that properly 2) Make --maximum mutualy exclusive with --current too 3) Allow --maximum and --current and document that it will fail for online domains I'm fine with either of those options There is also 4th option: specify that maximum requires config instead of make it mutually exclusive with current. This flag requirements are used by some of the other libvirt APIs. I'll create another set of macros to tell that some flags are required and send v2. Thanks for review. Pavel Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] qemu: fix set vcpus on host without NUMA
On Tue, Mar 24, 2015 at 05:38:37PM +0100, Peter Krempa wrote:
On Fri, Mar 20, 2015 at 15:39:04 +0100, Pavel Hrdina wrote:
We don't have to modify cpuset.mems on hosts without NUMA. It also
fixes an error message that you get instead of success if you trying
update vcpus of a guest on a host without NUMA.
Could you add example of the error you are fixing here?
Sure
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c4d96bd..eb86d68 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4889,7 +4889,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned
int nvcpus,
}
}
-if (flags VIR_DOMAIN_AFFECT_LIVE !(flags
VIR_DOMAIN_VCPU_GUEST)) {
+if (flags VIR_DOMAIN_AFFECT_LIVE !(flags VIR_DOMAIN_VCPU_GUEST)
+virNumaIsAvailable()) {
if (virCgroupNewEmulator(priv-cgroup, false, cgroup_temp) 0)
goto endjob;
ACK.
Peter
Thanks, will push it shortly.
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: change accidental VIR_WARNING back to VIR_DEBUG
On 03/24/2015 11:09 AM, John Ferlan wrote: On 03/24/2015 10:49 AM, Laine Stump wrote: While debugging the support for responding to qemu RX_FILTER_CHANGED events, I had changed the ignoring this event log message from VIR_DEBUG to VIR_WARN, but forgot to change it back before pushing. Since many guest OSes make enough changes to multicast lists and/or promiscuous mode settings to trigger this message, it's starting to show up as a red herring in bug reports. --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK - Ironically I had just started noticing this in a debug session and began wondering... At least you have the presence of mind to wonder, rather than blindly filing a bug report :-P Thanks for the ACK, I've pushed it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] qemu: cleanup setvcpus
On Tue, Mar 24, 2015 at 05:01:09PM +0100, Peter Krempa wrote: On Fri, Mar 20, 2015 at 15:39:01 +0100, Pavel Hrdina wrote: Remove unnecessary maximum variable and also exclusive flags check as they are now tested by upper layer for all drivers. The part about the exclusive flag check is not true. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_driver.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) ACK with commit message fixed. Peter Thanks, I'll push it shortly. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 3/6] Allocate virtio-serial addresses when starting a domain
Instead of always using controller 0 and incrementing port number,
respect the maximum port numbers of controllers and use all of them.
Ports for virtio consoles are quietly reserved, but not formatted
(neither in XML nor on QEMU command line).
Also rejects duplicate virtio-serial addresses.
https://bugzilla.redhat.com/show_bug.cgi?id=890606
https://bugzilla.redhat.com/show_bug.cgi?id=1076708
Test changes:
* virtio-auto.args
Filling out the port when just the controller is specified.
switched from using
maxport + 1
to:
first free port on the controller
* virtio-autoassign.args
Filling out the address when no address is specified.
Started using all the controllers instead of 0, also discards
the bus value.
* xml - xml output of virtio-auto
The port assignment is no longer done as a part of XML parsing,
so the unspecified values stay 0.
---
src/conf/domain_conf.c | 48 +
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c| 63 ++
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_process.c| 2 +
.../qemuxml2argv-channel-virtio-auto.args | 2 +-
.../qemuxml2argv-channel-virtio-autoassign.args| 10 ++--
.../qemuxml2xmlout-channel-virtio-auto.xml | 9 ++--
10 files changed, 93 insertions(+), 45 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d633f93..e777f5f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3498,21 +3498,6 @@
virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
chr-target.port = maxport + 1;
}
-
-if (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-chr-info.addr.vioserial.port == 0) {
-int maxport = 0;
-
-for (i = 0; i cnt; i++) {
-const virDomainChrDef *thischr = arrPtr[i];
-if (thischr-info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-thischr-info.addr.vioserial.controller ==
chr-info.addr.vioserial.controller
-thischr-info.addr.vioserial.bus ==
chr-info.addr.vioserial.bus
-(int)thischr-info.addr.vioserial.port maxport)
-maxport = thischr-info.addr.vioserial.port;
-}
-chr-info.addr.vioserial.port = maxport + 1;
-}
}
/* set default path for virtio-rng random backend to /dev/random */
@@ -12471,6 +12456,20 @@ virDomainControllerFind(virDomainDefPtr def,
}
int
+virDomainControllerFindByType(virDomainDefPtr def,
+ int type)
+{
+size_t i;
+
+for (i = 0; i def-ncontrollers; i++) {
+if (def-controllers[i]-type == type)
+return i;
+}
+
+return -1;
+}
+
+int
virDomainControllerFindByPCIAddress(virDomainDefPtr def,
virDevicePCIAddressPtr addr)
{
@@ -14906,25 +14905,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
def-channels[def-nchannels++] = chr;
-
-if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL
-chr-targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO
-chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
-chr-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL;
-
-if (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-chr-info.addr.vioserial.port == 0) {
-int maxport = 0;
-for (j = 0; j i; j++) {
-virDomainChrDefPtr thischr = def-channels[j];
-if (thischr-info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-thischr-info.addr.vioserial.controller ==
chr-info.addr.vioserial.controller
-thischr-info.addr.vioserial.bus ==
chr-info.addr.vioserial.bus
-(int)thischr-info.addr.vioserial.port maxport)
-maxport = thischr-info.addr.vioserial.port;
-}
-chr-info.addr.vioserial.port = maxport + 1;
-}
}
VIR_FREE(nodes);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bceb2d7..b756b40 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2701,6 +2701,7 @@ int virDomainControllerInsert(virDomainDefPtr def,
void virDomainControllerInsertPreAlloced(virDomainDefPtr def,
virDomainControllerDefPtr controller);
int virDomainControllerFind(virDomainDefPtr def, int type, int idx);
+int virDomainControllerFindByType(virDomainDefPtr def, int type);
int virDomainControllerFindByPCIAddress(virDomainDefPtr def,
virDevicePCIAddressPtr
[libvirt] [PATCHv3 4/6] Expand the address set when attaching a virtio-serial controller
---
src/qemu/qemu_hotplug.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9b8d11b..ef2f9b0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -437,6 +437,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr
driver,
char *devstr = NULL;
qemuDomainObjPrivatePtr priv = vm-privateData;
bool releaseaddr = false;
+bool addedToAddrSet = false;
if (virDomainControllerFind(vm-def, controller-type, controller-idx) =
0) {
virReportError(VIR_ERR_OPERATION_FAILED,
@@ -475,6 +476,12 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr
driver,
goto cleanup;
}
+if (controller-type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL
+virDomainVirtioSerialAddrSetAddController(priv-vioserialaddrs,
+ controller) 0)
+goto cleanup;
+addedToAddrSet = true;
+
if (!(devstr = qemuBuildControllerDevStr(vm-def, controller,
priv-qemuCaps, NULL)))
goto cleanup;
}
@@ -503,6 +510,9 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr
driver,
}
cleanup:
+if (ret != 0 addedToAddrSet)
+virDomainVirtioSerialAddrSetRemoveController(priv-vioserialaddrs,
+ controller);
if (ret != 0 releaseaddr)
qemuDomainReleaseDeviceAddress(vm, controller-info, NULL);
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] tools: fix the wrong check when use virsh setvcpus --maximum
On Fri, Mar 20, 2015 at 15:39:03 +0100, Pavel Hrdina wrote: From: Luyao Huang lhu...@redhat.com We will ignore --maximum option when only use setvcpus with this option, like this (this error is another issue): # virsh setvcpus test3 --maximum 10 error: Failed to create controller cpu for group: No such file or directory this is because we do not set it in flags before we check if there is a flags set. Refactor these code and fix the logic. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204033 Signed-off-by: Luyao Huang lhu...@redhat.com Signed-off-by: Pavel Hrdina phrd...@redhat.com --- tools/virsh-domain.c | 30 ++ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1d8225c..9430ad9 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6735,6 +6735,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); VSH_EXCLUSIVE_OPTIONS_VAR(guest, config); +VSH_EXCLUSIVE_OPTIONS_VAR(guest, maximum); +VSH_EXCLUSIVE_OPTIONS_VAR(config, maximum); As Yanbing pointed out, you want to make live and maximum exclusive. Additionally this changes semantics, as currently --maximum was possible if and only if --config was specified, which would make it exclusive with --current too. This is also implied in the man page. We have the following options: 1) Make --maximum imply --config and document that properly 2) Make --maximum mutualy exclusive with --current too 3) Allow --maximum and --current and document that it will fail for online domains I'm fine with either of those options Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] qemu: fix set vcpus on host without NUMA
On Fri, Mar 20, 2015 at 15:39:04 +0100, Pavel Hrdina wrote:
We don't have to modify cpuset.mems on hosts without NUMA. It also
fixes an error message that you get instead of success if you trying
update vcpus of a guest on a host without NUMA.
Could you add example of the error you are fixing here?
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c4d96bd..eb86d68 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4889,7 +4889,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
}
}
-if (flags VIR_DOMAIN_AFFECT_LIVE !(flags VIR_DOMAIN_VCPU_GUEST)) {
+if (flags VIR_DOMAIN_AFFECT_LIVE !(flags VIR_DOMAIN_VCPU_GUEST)
+virNumaIsAvailable()) {
if (virCgroupNewEmulator(priv-cgroup, false, cgroup_temp) 0)
goto endjob;
ACK.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Document behavior of compat when creating qcow2 volumes
On Tue, Mar 24, 2015 at 17:21:26 +0100, Ján Tomko wrote: Commit bab2eda changed the behavior for missing compat attribute, but failed to update the documentation. Before, the option was omitted from qemu-img command line and the qemu-img default was used. Now we always specify the compat value and the default is 0.10. Reported by Christophe Fergeau https://bugzilla.gnome.org/show_bug.cgi?id=746660#c4 --- docs/formatstorage.html.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index d2e2bb8..9c7b1bd 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -588,8 +588,9 @@ codetype='qcow2'/code volumes. Valid values are code0.10/code and code1.1/code so far, specifying QEMU version the images should be compatible with. If the codefeature/code element is present, -1.1 is used. If omitted, qemu-img default is used. -span class=sinceSince 1.1.0/span +1.1 is used. +span class=sinceSince 1.1.0/span If omitted, 0.10 is used. +span class=sinceSince 1.1.2/span You could explicitly state that previously the qemu-img default was used. /dd dtcodenocow/code/dt ddTurn off COW of the newly created volume. So far, this is only valida ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: add a max_core setting to qemu.conf for core dump size
On Tue, Mar 24, 2015 at 08:26:12 +0100, Ján Tomko wrote: On Mon, Mar 23, 2015 at 08:43:31PM -0400, John Ferlan wrote: On 03/18/2015 08:36 AM, Daniel P. Berrange wrote: Currently the QEMU processes inherit their core dump rlimit from libvirtd, which is really suboptimal. This change allows their limit to be directly controller from qemu.conf instead. --- src/libvirt_private.syms | 2 ++ src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 12 src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_process.c| 2 ++ src/qemu/test_libvirtd_qemu.aug.in | 1 + src/util/vircommand.c | 14 ++ src/util/vircommand.h | 1 + src/util/virprocess.c | 35 +++ src/util/virprocess.h | 1 + 11 files changed, 74 insertions(+) ... diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 1c589a2..12e4326 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -390,6 +390,18 @@ #max_processes = 0 #max_files = 0 +# If max_core is set to a positive integer, then QEMU will be +# permitted to create core dumps when it crashes, provided its +# RAM size is smaller than the limit set. Be warned that the +# core dump will include a full copy of the guest RAM, so if +# the largest guest is 32 GB in size, the max_core limit will +# have to be at least 33/34 GB to allow enough overhead. +# +# By default it will inherit core limit from libvirtd, which +# is usually set to 0 by systemd/init. +# +# Size is in bytes +#max_core = 0 It's too bad it cannot be a sized value... Reading 20G in a file for me is so much easier than the comparable byte value say nothing if we get to 128G, 1T, etc. (some day). Having the option as a string would also allow non-integer values, like unlimited. I definitely vote for an option to set unlimited rather than having to specify a large number to achieve the same effect. Jan Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] internal: introduce macro helpers to reject exclusive flags
On Tue, Mar 24, 2015 at 04:40:17PM +0100, Peter Krempa wrote:
On Fri, Mar 20, 2015 at 15:38:59 +0100, Pavel Hrdina wrote:
Inspired by commit 7e437ee7 that introduced similar macros for virsh
commands so we don't have to repeat the same code all over.
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/internal.h | 80
++
1 file changed, 80 insertions(+)
diff --git a/src/internal.h b/src/internal.h
index 4d473af..5885f03 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -327,6 +327,86 @@
} \
} while (0)
+/* Macros to help dealing with mutually exclusive flags. */
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_EXPR_RET:
+ *
+ * @NAME1: String containing the name of the flag.
+ * @EXPR1: Expression to validate the flag.
+ * @NAME2: String containing the name of the flag.
+ * @EXPR2: Expression to validate the flag.
+ *
+ * Reject mutually exclusive API flags. Use the provided expression
+ * to check the flags.
+ *
+ * This helper does an early return and therefore it has to be called
+ * before anything that would require cleanup.
+ */
+# define VIR_EXCLUSIVE_FLAGS_EXPR_RET(NAME1, EXPR1, NAME2, EXPR2)
\
+if ((EXPR1) (EXPR2)) {
\
+virReportInvalidArg(ctl,
\
+_(Flags '%s' and '%s' are mutually
exclusive),\
+NAME1, NAME2);
\
+return -1;
\
+}
+
While in virsh the above variant makes sense, because in some cases the
variables have different names than the corresponding flags, in case of
this code having it doesn't make sense.
You're right. I've only used the other macros.
+/**
+ * VIR_EXCLUSIVE_FLAGS_VAR_RET:
+ *
+ * @FLAG1: First flag to be checked
+ * @FLAG2: Second flag to be checked
A third argument @RET should be added here so that this can be used in
places that also return NULL or any other possible variable.
In virsh the assumption is that the flags are checked at first and thus
returning false makes sense, as every virsh command does that.
I'll add the @RET argument.
+ *
+ * Reject mutually exclusive API flags. The checked flags are compared
+ * with flags variable.
+ *
+ * This helper does an early return and therefore it has to be called
+ * before anything that would require cleanup.
+ */
+# define VIR_EXCLUSIVE_FLAGS_RET(FLAG1, FLAG2)
\
+VIR_EXCLUSIVE_FLAGS_EXPR_RET(#FLAG1, flags FLAG1, #FLAG2, flags
FLAG2)
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_EXPR_GOTO:
+ *
+ * @NAME1: String containing the name of the flag.
+ * @EXPR1: Expression to validate the flag.
+ * @NAME2: String containing the name of the flag.
+ * @EXPR2: Expression to validate the flag.
+ * @LABEL: label to jump to.
+ *
+ * Reject mutually exclusive API flags. Use the provided expression
+ * to check the flag.
+ *
+ * Returns nothing. Jumps to a label if unsupported flags were
+ * passed to it.
+ */
+# define VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(NAME1, EXPR1, NAME2, EXPR2, LABEL)
\
+if ((EXPR1) (EXPR2)) {
\
+virReportInvalidArg(ctl,
\
+_(Flags '%s' and '%s' are mutually
exclusive),\
+NAME1, NAME2);
\
+goto LABEL;
\
+}
+
Again, this function does not make sense in the library code.
+/**
+ * VIR_EXCLUSIVE_FLAGS_VAR_GOTO:
+ *
+ * @FLAG1: First flag to be checked.
+ * @FLAG2: Second flag to be checked.
+ * @LABEL: label to jump to.
+ *
+ * Reject mutually exclusive API flags. The checked flags are compared
+ * with flags variable.
+ *
+ * Returns nothing. Jumps to a label if unsupported flags were
+ * passed to it.
+ */
+# define VIR_EXCLUSIVE_FLAGS_GOTO(FLAG1, FLAG2, LABEL)
\
+VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(#FLAG1, flags FLAG1,
\
+ #FLAG2, flags FLAG2, LABEL)
+
+
# define virCheckNonNullArgReturn(argname, retval) \
do {\
if (argname == NULL) { \
While this is okay.
Peter
Thanks for review, I'll send v2.
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Document behavior of compat when creating qcow2 volumes
On Tue, Mar 24, 2015 at 05:43:41PM +0100, Peter Krempa wrote: On Tue, Mar 24, 2015 at 17:21:26 +0100, Ján Tomko wrote: Commit bab2eda changed the behavior for missing compat attribute, but failed to update the documentation. Before, the option was omitted from qemu-img command line and the qemu-img default was used. Now we always specify the compat value and the default is 0.10. Reported by Christophe Fergeau https://bugzilla.gnome.org/show_bug.cgi?id=746660#c4 --- docs/formatstorage.html.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index d2e2bb8..9c7b1bd 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -588,8 +588,9 @@ codetype='qcow2'/code volumes. Valid values are code0.10/code and code1.1/code so far, specifying QEMU version the images should be compatible with. If the codefeature/code element is present, -1.1 is used. If omitted, qemu-img default is used. -span class=sinceSince 1.1.0/span +1.1 is used. +span class=sinceSince 1.1.0/span If omitted, 0.10 is used. +span class=sinceSince 1.1.2/span You could explicitly state that previously the qemu-img default was used. I didn't want to clutter the documentation by mentioning behavior that was only there for two releases over a year ago. The version gap there should be enough for someone using libvirt 1.1.0 that the behavior with the missing attribute is unspecified. /dd dtcodenocow/code/dt ddTurn off COW of the newly created volume. So far, this is only valida ACK, Thanks, pushed. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/6] qemu: move virDomainLiveConfigHelperMethod right after BeginJob
On Fri, Mar 20, 2015 at 15:58:56 +0100, Pavel Hrdina wrote: We should call virDomainLiveConfigHelperMethod ASAP because this function transfers VIR_DOMAIN_AFFECT_CURRENT to VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. All other additional checks for those two flags should consider that the user give us VIR_DOMAIN_AFFECT_CURRENT. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_driver.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) ACK, although this should be 0.5/6 or 1.5/6. Basically just push this first. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Document behavior of compat when creating qcow2 volumes
Commit bab2eda changed the behavior for missing compat attribute, but failed to update the documentation. Before, the option was omitted from qemu-img command line and the qemu-img default was used. Now we always specify the compat value and the default is 0.10. Reported by Christophe Fergeau https://bugzilla.gnome.org/show_bug.cgi?id=746660#c4 --- docs/formatstorage.html.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index d2e2bb8..9c7b1bd 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -588,8 +588,9 @@ codetype='qcow2'/code volumes. Valid values are code0.10/code and code1.1/code so far, specifying QEMU version the images should be compatible with. If the codefeature/code element is present, -1.1 is used. If omitted, qemu-img default is used. -span class=sinceSince 1.1.0/span +1.1 is used. +span class=sinceSince 1.1.0/span If omitted, 0.10 is used. +span class=sinceSince 1.1.2/span /dd dtcodenocow/code/dt ddTurn off COW of the newly created volume. So far, this is only valid -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] use new macro helpers to check exclusive flags
On Fri, Mar 20, 2015 at 15:39:00 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/libvirt-domain-snapshot.c | 45 ++
src/libvirt-domain.c | 288
+++--
src/qemu/qemu_driver.c | 9 +-
src/storage/storage_backend_disk.c | 10 +-
src/storage/storage_backend_fs.c | 11 +-
5 files changed, 106 insertions(+), 257 deletions(-)
@@ -7340,14 +7252,21 @@ virDomainSetVcpusFlags(virDomainPtr domain, unsigned
int nvcpus,
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain-conn-flags, error);
-if (flags VIR_DOMAIN_VCPU_GUEST
-flags VIR_DOMAIN_VCPU_MAXIMUM) {
-virReportInvalidArg(flags,
-_(flags 'VIR_DOMAIN_VCPU_MAXIMUM' and
- 'VIR_DOMAIN_VCPU_GUEST' in '%s' are mutually
- exclusive), __FUNCTION__);
-goto error;
-}
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
+ VIR_DOMAIN_AFFECT_LIVE,
+ error);
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_VCPU_GUEST,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_VCPU_GUEST,
+ VIR_DOMAIN_VCPU_MAXIMUM,
+ error);
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CONFIG,
+ VIR_DOMAIN_VCPU_MAXIMUM,
+ error);
By the way, this check here is not enough to check that MAXIMUM is not
actually used with _AFFECT_LIVE.
If you use VIR_DOMAIN_AFFECT_CURRENT and the guest is online, this check
is bypassed as the state of the domain is not known at this point.
This unfortunately needs to be checked after the
virDomainLiveConfigHelperMethod in the actual code.
virCheckNonZeroArgGoto(nvcpus, error);
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 2/6] Add functions to track virtio-serial addresses
Create a sorted array of virtio-serial controllers.
Each of the elements contains the controller index
and a bitmap of available ports.
Buses are not tracked, because they aren't supported by QEMU.
---
src/conf/domain_addr.c | 398 +++
src/conf/domain_addr.h | 59 +++
src/libvirt_private.syms | 10 ++
3 files changed, 467 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index fb4a76f..49d28be 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -718,3 +718,401 @@ virDomainCCWAddressSetCreate(void)
virDomainCCWAddressSetFree(addrs);
return NULL;
}
+
+
+#define VIR_DOMAIN_DEFAULT_VIRTIO_SERIAL_PORTS 31
+
+
+/* virDomainVirtioSerialAddrSetCreate
+ *
+ * Allocates an address set for virtio serial addresses
+ */
+virDomainVirtioSerialAddrSetPtr
+virDomainVirtioSerialAddrSetCreate(void)
+{
+virDomainVirtioSerialAddrSetPtr ret = NULL;
+
+if (VIR_ALLOC(ret) 0)
+return NULL;
+
+return ret;
+}
+
+static void
+virDomainVirtioSerialControllerFree(virDomainVirtioSerialControllerPtr cont)
+{
+if (cont) {
+virBitmapFree(cont-ports);
+VIR_FREE(cont);
+}
+}
+
+static ssize_t
+virDomainVirtioSerialAddrPlaceController(virDomainVirtioSerialAddrSetPtr addrs,
+ virDomainVirtioSerialControllerPtr
cont)
+{
+size_t i;
+
+for (i = 0; i addrs-ncontrollers; i++) {
+if (addrs-controllers[i]-idx == cont-idx) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(virtio serial controller with index %u already
exists
+ in the address set),
+ cont-idx);
+return -2;
+}
+if (addrs-controllers[i]-idx cont-idx)
+return i;
+}
+return -1;
+}
+
+static ssize_t
+virDomainVirtioSerialAddrFindController(virDomainVirtioSerialAddrSetPtr addrs,
+unsigned int idx)
+{
+size_t i;
+
+for (i = 0; i addrs-ncontrollers; i++) {
+if (addrs-controllers[i]-idx == idx)
+return i;
+}
+return -1;
+}
+
+/* virDomainVirtioSerialAddrSetAddController
+ *
+ * Adds virtio serial ports of the existing controller
+ * to the address set.
+ */
+int
+virDomainVirtioSerialAddrSetAddController(virDomainVirtioSerialAddrSetPtr
addrs,
+ virDomainControllerDefPtr cont)
+{
+int ret = -1;
+int ports;
+virDomainVirtioSerialControllerPtr cnt = NULL;
+ssize_t insertAt;
+
+if (cont-type != VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
+return 0;
+
+ports = cont-opts.vioserial.ports;
+if (ports == -1)
+ports = VIR_DOMAIN_DEFAULT_VIRTIO_SERIAL_PORTS;
+
+VIR_DEBUG(Adding virtio serial controller index %u with %d
+ ports to the address set, cont-idx, ports);
+
+if (VIR_ALLOC(cnt) 0)
+goto cleanup;
+
+if (!(cnt-ports = virBitmapNew(ports)))
+goto cleanup;
+cnt-idx = cont-idx;
+
+if ((insertAt = virDomainVirtioSerialAddrPlaceController(addrs, cnt)) -1)
+goto cleanup;
+if (VIR_INSERT_ELEMENT(addrs-controllers, insertAt,
+ addrs-ncontrollers, cnt) 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virDomainVirtioSerialControllerFree(cnt);
+return ret;
+}
+
+/* virDomainVirtioSerialAddrSetAddControllers
+ *
+ * Adds virtio serial ports of controllers present in the domain definition
+ * to the address set.
+ */
+int
+virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr
addrs,
+ virDomainDefPtr def)
+{
+size_t i;
+
+for (i = 0; i def-ncontrollers; i++) {
+if (virDomainVirtioSerialAddrSetAddController(addrs,
+ def-controllers[i]) 0)
+return -1;
+}
+
+return 0;
+}
+
+/* virDomainVirtioSerialAddrSetRemoveController
+ *
+ * Removes a virtio serial controller from the address set.
+ */
+void
+virDomainVirtioSerialAddrSetRemoveController(virDomainVirtioSerialAddrSetPtr
addrs,
+ virDomainControllerDefPtr cont)
+{
+ssize_t pos;
+
+if (cont-type != VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
+return;
+
+VIR_DEBUG(Removing virtio serial controller index %u
+ from the address set, cont-idx);
+
+pos = virDomainVirtioSerialAddrFindController(addrs, cont-idx);
+
+if (pos = 0)
+VIR_DELETE_ELEMENT(addrs-controllers, pos, addrs-ncontrollers);
+}
+
+void
+virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs)
+{
+size_t i;
+if (addrs) {
+for (i = 0; i addrs-ncontrollers; i++)
+virDomainVirtioSerialControllerFree(addrs-controllers[i]);
+VIR_FREE(addrs);
+}
+}
+
+static int
[libvirt] [PATCHv3 0/6] Allocate virtio-serial addresses
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1076708 https://bugzilla.redhat.com/show_bug.cgi?id=890606 New in v3: * preserve the behavior of honoring the controller if a partial address is specified: address type='virtio-serial' controller='2'/ * automatically add a controller if we're out of free ports and add a test for it Ján Tomko (6): Add test for virtio serial port assignment Add functions to track virtio-serial addresses Allocate virtio-serial addresses when starting a domain Expand the address set when attaching a virtio-serial controller Assign an address when hotplugging a virtio-serial device Auto add virtio-serial controllers src/conf/domain_addr.c | 435 + src/conf/domain_addr.h | 61 +++ src/conf/domain_conf.c | 48 +-- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 11 + src/qemu/qemu_command.c| 63 +++ src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_hotplug.c| 32 +- src/qemu/qemu_process.c| 2 + tests/qemuhotplugtest.c| 2 +- .../qemuxml2argv-channel-virtio-auto.args | 2 +- .../qemuxml2argv-channel-virtio-autoadd.args | 33 ++ .../qemuxml2argv-channel-virtio-autoadd.xml| 45 +++ .../qemuxml2argv-channel-virtio-autoassign.args| 20 + .../qemuxml2argv-channel-virtio-autoassign.xml | 50 +++ tests/qemuxml2argvtest.c | 4 + .../qemuxml2xmlout-channel-virtio-auto.xml | 9 +- 18 files changed, 777 insertions(+), 43 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoadd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoadd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] qemu_migration: Precreate missing storage breaks with network drives
In our case the network disk is available on the dest before we call migration. It's an openstack volume mounted via ceph. If we disable the copy-check in qemu_migration.c the migration works fine. On Mon, Mar 23, 2015 at 12:17 AM, Michal Privoznik mpriv...@redhat.com wrote: On 20.03.2015 20:23, Noel Burton-Krahn wrote: Hi Michal, I think issuing a libvirt migrate to a host where the network disks don't already exist would be a prequisite failure. Libvirt can never copy a network disk, but it shouldn't fail trying to migrate an existing domain that contains a network disk. If a libvirt user wishes to migrate a domain that contains a network disk, it's their responsibility to ensure the disk exists before calling migrate. That's how it was back in the old days. Then libvirt introduced storage migration, but requiring users to pre-create storage on destination themselves. And just recently I taught libvirt to automatically pre-create the storage. However, not for all disk types. If a disk on destination is missing, it's likely broken guest ABI libvirt should have not allowed migration in the first place. How come the migration was allowed? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] qemu: check if domain is really active when do setvcpus with --live
On 03/20/2015 10:39 PM, Pavel Hrdina wrote:
From: Luyao Huang lhu...@redhat.com
Commit e105dc9 fix setting vcpus for offline domain, but forget check
if vm is active when pass VIR_DOMAIN_AFFECT_LIVE flags.
# virsh setvcpus test3 4 --live
error: Failed to create controller cpu for group: No such file or directory
Add a check if we pass VIR_DOMAIN_AFFECT_LIVE flags.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204006
Signed-off-by: Luyao Huang lhu...@redhat.com
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_driver.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4942712..c4d96bd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4881,6 +4881,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup;
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(domain is not running));
+goto endjob;
+}
+}
+
if (flags VIR_DOMAIN_AFFECT_LIVE !(flags VIR_DOMAIN_VCPU_GUEST)) {
if (virCgroupNewEmulator(priv-cgroup, false, cgroup_temp) 0)
goto endjob;
@@ -4947,12 +4955,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
if (ncpuinfo 0)
goto endjob;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain is not running));
-goto endjob;
-}
-
if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) 0)
goto endjob;
Sorry, i forgot there is a function virDomainLiveConfigHelperMethod(),
move this function just after qemuDomainObjBeginJob() maybe a good way
to fix this issue. Also virDomainLiveConfigHelperMethod() may change
flags, so it should be done more early (as soon as possible after set a
lock to vm).
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/2] network_conf: check if bridge exists on host for user created bridges
On 03/24/2015 11:12 AM, Laine Stump wrote:
On 03/24/2015 05:59 AM, Shivaprasad G Bhat wrote:
virNetworkBridgeInUse() doesn't check if the bridge is manually created
outside of libvirt. Check if the bridge actually exist on host using the
virNetDevExists().
Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
---
src/conf/network_conf.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d7c5dec..c3ae2e4 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3227,13 +3227,22 @@ virNetworkBridgeInUseHelper(const void *payload,
int ret = 0;
virNetworkObjPtr net = (virNetworkObjPtr) payload;
const struct virNetworkBridgeInUseHelperData *data = opaque;
+bool defined_bridge = false;
virObjectLock(net);
if (net-def-bridge
-STREQ(net-def-bridge, data-bridge)
-!(data-skipname STREQ(net-def-name, data-skipname)))
-ret = 1;
+STREQ(net-def-bridge, data-bridge)) {
+defined_bridge = true;
+if (!(data-skipname STREQ(net-def-name, data-skipname)))
+ ret = 1;
+}
+
virObjectUnlock(net);
+
+/* Bridge might have been created by a user manually outside libvirt */
+if (!defined_bridge !ret)
+ret = virNetDevExists(data-bridge) ? 1 : 0;
+
return ret;
}
This function is intended to be called once for each defined network on
the host, with data-bridge being the same each time, but
net-def-bridge being different, so adding the check for data-bridge
existence here may work, but it's a bit convoluted.
Instead, you should leave this function alone, and just add the extra
check directly in the function virNetworkBridgeInUse() (either before
locking, or after unlocking nets).
You know, there's another problem with this - adding this call to
virNetDevExists() would be the first case of anything in the conf
directory (which is supposed to be platform-independent
parsing/formatting of XML and maintenance of lists of config objects)
that calls a virNetDev*() function which is dependent on the current
platform (and having root privileges). For the current uses of
virNetworkBridgeInUse() it ends up not being a problem, because the only
caller of virNetworkBridgeInUse() will already be verified as running on
a platform that supports the virNetDev* functions and having root
privileges (and/or certain to fail if we made this call on return), but
it still leaves a bad taste in my mouth to be calling a device ioctl
from a function in the conf directory.
The trouble of course is that doing it differently turns this into a
much bigger problem - as it is network_conf.c mostly isolates the
network driver (bridge_driver.c) from the idea that a network could be
defined without a bridge name specified, or that there might be a
conflicting bridge name, by calling virNetworkSetBridgeName() which is
in network_conf.c (it also calls virNetworkSetBridgeName() on its own
when loading the network configs from disk, thus not even giving the
network driver a chance to intervene).
So I don't want to be the one to NACK based on this cross pollination,
but thought I should point it out in case anyone with a stronger opinion
did (and even if not, so that we see that even if we accept a patch like
the current one to fix the bug now, we may still want to plan a
different fix in the long run. Or maybe not - maybe I'm being too pedantic).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: fix dom0 balloon logic
Stefano Stabellini wrote:
On Mon, 23 Mar 2015, Ian Campbell wrote:
(just ccing the other tools maintainers, in particular Stefano who knows
what this stuff is supposed to do...)
On Fri, 2015-03-20 at 17:10 -0600, Jim Fehlig wrote:
Recent testing on large memory systems revealed a bug in the Xen xl
tool's freemem() function. When autoballooning is enabled, freemem()
is used to ensure enough memory is available to start a domain,
ballooning dom0 if necessary. When ballooning large amounts of memory
from dom0, freemem() would exceed its self-imposed wait time and
return an error. Meanwhile, dom0 continued to balloon. Starting the
domain later, after sufficient memory was ballooned from dom0, would
succeed. The libvirt implementation in libxlDomainFreeMem() suffers
the same bug since it is modeled after freemem().
In the end, the best place to fix the bug on the Xen side was to
slightly change the behavior of libxl_wait_for_memory_target().
Instead of failing after caller-provided wait_sec, the function now
blocks as long as dom0 memory ballooning is progressing. It will return
failure only when more memory is needed to reach the target and wait_sec
have expired with no progress being made. See xen.git commit fd3aa246.
There was a dicussion on how this would affect other libxl apps like
libvirt
http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html
If libvirt containing this patch was build against a Xen containing
the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem()
will fail after 30 sec and domain creation will be terminated.
Without this patch and with old libxl_wait_for_memory_target() behavior,
libxlDomainFreeMem() does not succeed after 30 sec, but returns success
anyway. Domain creation continues resulting in all sorts of fun stuff
like cpu soft lockups in the guest OS. It was decided to properly fix
libxl_wait_for_memory_target(), and if anything improve the default
behavior of apps using the freemem reference impl in xl.
xl was patched to accommodate the change in libxl_wait_for_memory_target()
with xen.git commit 883b30a0. This patch does the same in the libxl
driver. While at it, I changed the logic to essentially match
freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner
IMO and will make it easier to spot future, potentially interesting
divergences.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/libxl/libxl_domain.c | 57
1 file changed, 28 insertions(+), 29 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 407a9bd..ff78133 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1121,38 +1121,41 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv,
libxl_domain_config *d_config)
{
uint32_t needed_mem;
uint32_t free_mem;
-size_t i;
-int ret = -1;
+int ret;
int tries = 3;
int wait_secs = 10;
-if ((ret = libxl_domain_need_memory(priv-ctx, d_config-b_info,
-needed_mem)) = 0) {
-for (i = 0; i tries; ++i) {
-if ((ret = libxl_get_free_memory(priv-ctx, free_mem)) 0)
-break;
+ret = libxl_domain_need_memory(priv-ctx, d_config-b_info,
+ needed_mem);
+if (ret 0)
+goto error;
-if (free_mem = needed_mem) {
-ret = 0;
-break;
-}
+do {
+ret = libxl_get_free_memory(priv-ctx, free_mem);
+if (ret 0)
+goto error;
-if ((ret = libxl_set_memory_target(priv-ctx, 0,
- free_mem - needed_mem,
- /* relative */ 1, 0)) 0)
-break;
+if (free_mem = needed_mem)
+return 0;
-ret = libxl_wait_for_free_memory(priv-ctx, 0, needed_mem,
- wait_secs);
-if (ret == 0 || ret != ERROR_NOMEM)
-break;
+ret = libxl_set_memory_target(priv-ctx, 0,
+ free_mem - needed_mem,
+ /* relative */ 1, 0);
+if (ret 0)
+goto error;
-if ((ret = libxl_wait_for_memory_target(priv-ctx, 0, 1)) 0)
-break;
-}
-}
+ret = libxl_wait_for_free_memory(priv-ctx, 0, needed_mem,
+ wait_secs);
+if (ret 0)
+goto error;
Shouldn't this be a call to libxl_wait_for_memory_target instead?
Err, right. One would think I would have caught that after all the talk
about libxl_wait_for_memory_target() in the commit msg :-/. V2 on the way...
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
[libvirt] [PATCH V2] libxl: fix dom0 balloon logic
Recent testing on large memory systems revealed a bug in the Xen xl
tool's freemem() function. When autoballooning is enabled, freemem()
is used to ensure enough memory is available to start a domain,
ballooning dom0 if necessary. When ballooning large amounts of memory
from dom0, freemem() would exceed its self-imposed wait time and
return an error. Meanwhile, dom0 continued to balloon. Starting the
domain later, after sufficient memory was ballooned from dom0, would
succeed. The libvirt implementation in libxlDomainFreeMem() suffers
the same bug since it is modeled after freemem().
In the end, the best place to fix the bug on the Xen side was to
slightly change the behavior of libxl_wait_for_memory_target().
Instead of failing after caller-provided wait_sec, the function now
blocks as long as dom0 memory ballooning is progressing. It will return
failure only when more memory is needed to reach the target and wait_sec
have expired with no progress being made. See xen.git commit fd3aa246.
There was a dicussion on how this would affect other libxl apps like
libvirt
http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html
If libvirt containing this patch was build against a Xen containing
the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem()
will fail after 30 sec and domain creation will be terminated.
Without this patch and with old libxl_wait_for_memory_target() behavior,
libxlDomainFreeMem() does not succeed after 30 sec, but returns success
anyway. Domain creation continues resulting in all sorts of fun stuff
like cpu soft lockups in the guest OS. It was decided to properly fix
libxl_wait_for_memory_target(), and if anything improve the default
behavior of apps using the freemem reference impl in xl.
xl was patched to accommodate the change in libxl_wait_for_memory_target()
with xen.git commit 883b30a0. This patch does the same in the libxl
driver. While at it, I changed the logic to essentially match
freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner
IMO and will make it easier to spot future, potentially interesting
divergences.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
V2: Actually use libxl_wait_for_memory_target(), instead of
libxl_wait_for_free_memory()
src/libxl/libxl_domain.c | 55 +++-
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 407a9bd..a1739aa 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1121,38 +1121,39 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv,
libxl_domain_config *d_config)
{
uint32_t needed_mem;
uint32_t free_mem;
-size_t i;
-int ret = -1;
+int ret;
int tries = 3;
int wait_secs = 10;
-if ((ret = libxl_domain_need_memory(priv-ctx, d_config-b_info,
-needed_mem)) = 0) {
-for (i = 0; i tries; ++i) {
-if ((ret = libxl_get_free_memory(priv-ctx, free_mem)) 0)
-break;
+ret = libxl_domain_need_memory(priv-ctx, d_config-b_info, needed_mem);
+if (ret 0)
+goto error;
-if (free_mem = needed_mem) {
-ret = 0;
-break;
-}
+do {
+ret = libxl_get_free_memory(priv-ctx, free_mem);
+if (ret 0)
+goto error;
-if ((ret = libxl_set_memory_target(priv-ctx, 0,
- free_mem - needed_mem,
- /* relative */ 1, 0)) 0)
-break;
+if (free_mem = needed_mem)
+return 0;
-ret = libxl_wait_for_free_memory(priv-ctx, 0, needed_mem,
- wait_secs);
-if (ret == 0 || ret != ERROR_NOMEM)
-break;
+ret = libxl_set_memory_target(priv-ctx, 0,
+ free_mem - needed_mem,
+ /* relative */ 1, 0);
+if (ret 0)
+goto error;
-if ((ret = libxl_wait_for_memory_target(priv-ctx, 0, 1)) 0)
-break;
-}
-}
+ret = libxl_wait_for_memory_target(priv-ctx, 0, wait_secs);
+if (ret 0)
+goto error;
-return ret;
+tries--;
+} while (tries 0);
+
+ error:
+virReportSystemError(ret, %s,
+ _(Failed to balloon domain0 memory));
+return -1;
}
static void
@@ -1271,12 +1272,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
priv-ctx, d_config) 0)
goto endjob;
-if (cfg-autoballoon libxlDomainFreeMem(priv, d_config) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(libxenlight failed to get free memory for domain
'%s'),
- d_config.c_info.name);
+if
Re: [libvirt] [RFC PATCH v2 00/12] qemu: add support to hot-plug/unplug cpu device
HI, Do you have plan to update this patchset based on the comments recently or have the latest one to post? I noticed that the patchset for memory hotplug had got merged, is there any plan to merge this patchset? On Fri, Feb 13, 2015 at 12:13 AM, Peter Krempa pkre...@redhat.com wrote: On Thu, Feb 12, 2015 at 19:53:13 +0800, Zhu Guihua wrote: Hi all, Any comments about this series? I'm not sure whether this series' method to support cpu hotplug in libvirt is reasonable, so could anyone give more suggestions about this function? Thanks. Well, as Dan pointed out in his review for this series and previous version, we are not convinced that we need a way to specify the CPU model and other parameters as having dissimilar CPUs doesn't make sense. A solution is either to build the cpu plug on top of the existing API or provide enough information to convince us that having the cpu model in the XML actually makes sense. Regards, Zhu Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Regards, Zhi Yong Wu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] Make debugging of cannot acquire state change lock easier
On 03/23/2015 09:25 AM, Jiri Denemark wrote: https://bugzilla.redhat.com/show_bug.cgi?id=853839 Jiri Denemark (8): POTFILES.in: Sort Add support for tracking thread jobs Force usage of virThreadCreate virThread: Set thread job virThreadPool: Set thread worker name Set thread job for every RPC call qemu: Track the API which started the current job qemu: Add timing to domain jobs cfg.mk | 9 +++ daemon/remote.c | 1 + include/libvirt/virterror.h | 1 + po/POTFILES.in | 5 +- src/Makefile.am | 2 + src/libvirt_private.syms| 11 +++- src/locking/lock_daemon_dispatch.c | 1 + src/nwfilter/nwfilter_learnipaddr.c | 15 ++--- src/nwfilter/nwfilter_learnipaddr.h | 1 - src/qemu/qemu_domain.c | 60 ++--- src/qemu/qemu_domain.h | 4 ++ src/rpc/gendispatch.pl | 6 +- src/util/virerror.c | 1 + src/util/virthread.c| 25 +-- src/util/virthread.h| 13 ++-- src/util/virthreadjob.c | 126 src/util/virthreadjob.h | 33 ++ src/util/virthreadpool.c| 44 - src/util/virthreadpool.h| 14 ++-- 19 files changed, 317 insertions(+), 55 deletions(-) create mode 100644 src/util/virthreadjob.c create mode 100644 src/util/virthreadjob.h Looks fine to me... Couple of 'overall' comments... * Clean run through my Coverity checker. * New modules add copyright for 2013-2015 - shouldn't that just be 2015 * Other modules touched haven't had their copyrights adjusted... I don't always remember either - perhaps everyone needs eblake's emacs macro to auto update when you edit a file. ACK series John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: add a max_core setting to qemu.conf for core dump size
On Mon, Mar 23, 2015 at 08:43:31PM -0400, John Ferlan wrote: On 03/18/2015 08:36 AM, Daniel P. Berrange wrote: Currently the QEMU processes inherit their core dump rlimit from libvirtd, which is really suboptimal. This change allows their limit to be directly controller from qemu.conf instead. --- src/libvirt_private.syms | 2 ++ src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 12 src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_process.c| 2 ++ src/qemu/test_libvirtd_qemu.aug.in | 1 + src/util/vircommand.c | 14 ++ src/util/vircommand.h | 1 + src/util/virprocess.c | 35 +++ src/util/virprocess.h | 1 + 11 files changed, 74 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ca3520d..7446357 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1240,6 +1240,7 @@ virCommandSetErrorFD; virCommandSetGID; virCommandSetInputBuffer; virCommandSetInputFD; +virCommandSetMaxCoreSize; virCommandSetMaxFiles; virCommandSetMaxMemLock; virCommandSetMaxProcesses; @@ -1951,6 +1952,7 @@ virProcessRunInMountNamespace; virProcessSchedPolicyTypeFromString; virProcessSchedPolicyTypeToString; virProcessSetAffinity; +virProcessSetMaxCoreSize; virProcessSetMaxFiles; virProcessSetMaxMemLock; virProcessSetMaxProcesses; diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 62951da..029a55a 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -71,6 +71,7 @@ module Libvirtd_qemu = | bool_entry set_process_name | int_entry max_processes | int_entry max_files + | int_entry max_core let device_entry = bool_entry mac_filter | bool_entry relaxed_acs_check diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 1c589a2..12e4326 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -390,6 +390,18 @@ #max_processes = 0 #max_files = 0 +# If max_core is set to a positive integer, then QEMU will be +# permitted to create core dumps when it crashes, provided its +# RAM size is smaller than the limit set. Be warned that the +# core dump will include a full copy of the guest RAM, so if +# the largest guest is 32 GB in size, the max_core limit will +# have to be at least 33/34 GB to allow enough overhead. +# +# By default it will inherit core limit from libvirtd, which +# is usually set to 0 by systemd/init. +# +# Size is in bytes +#max_core = 0 It's too bad it cannot be a sized value... Reading 20G in a file for me is so much easier than the comparable byte value say nothing if we get to 128G, 1T, etc. (some day). Having the option as a string would also allow non-integer values, like unlimited. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Document that USB hostdevs do not need nodeDettach
On 03/23/2015 01:16 PM, Ján Tomko wrote: The virNodeDeviceDettach API only works on PCI devices. Originally added by commit 10d3272e, but the API never supported USB devices. Reported by: Martin Polednik mpoled...@redhat.com --- docs/formatdomain.html.in | 19 +-- tools/virsh.pod | 17 - 2 files changed, 17 insertions(+), 19 deletions(-) ACK - Should we say to call virNodeDeviceDetachFlags instead of virNodeDeviceDettach? John diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 82aa14f..d6abe17 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3114,21 +3114,20 @@ with additional attributes noted. dl dtusb/dt - ddFor USB devices, the user is responsible to call -codevirNodeDeviceDettach/code (or -codevirsh nodedev-detach/code) before starting the guest -or hot-plugging the device and codevirNodeDeviceReAttach/code -(or codevirsh nodedev-reattach/code) after hot-unplug or -stopping the guest. + ddUSB devices are detached from the host on guest startup +and reattached after the guest exits or the device is +hot-unplugged. /dd dtpci/dt ddFor PCI devices, when codemanaged/code is yes it is detached from the host before being passed on to the guest and reattached to the host after the guest exits. If -codemanaged/code is omitted or no, follow the steps -described for a USB device to detach before starting the -guest or hot-plugging and reattach after stopping the guest -or hot-unplug. +codemanaged/code is omitted or no, the user is +responsible to call codevirNodeDeviceDettach/code +(or codevirsh nodedev-detach/code before starting the guest +or hot-plugging the device and codevirNodeDeviceReAttach/code +(or codevirsh nodedev-reattach/code) after hot-unplug or +stopping the guest. /dd dtscsi/dt ddFor SCSI devices, user is responsible to make sure the device diff --git a/tools/virsh.pod b/tools/virsh.pod index 8262a45..4d825c1 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2385,7 +2385,7 @@ attach taking effect the next time libvirt starts the domain. For cdrom and floppy devices, this command only replaces the media within an existing device; consider using Bupdate-device for this usage. For passthrough host devices, see also Bnodedev-detach, -needed if the device does not use managed mode. +needed if the PCI device does not use managed mode. If I--live is specified, affect a running domain. If I--config is specified, affect the next startup of a persistent domain. @@ -2646,15 +2646,14 @@ Lhttp://libvirt.org/formatnode.html. Passthrough devices cannot be simultaneously used by the host and its guest domains, nor by multiple active guests at once. If the -hostdev description includes the attribute Bmanaged='yes', and the -hypervisor driver supports it, then the device is in managed mode, and +hostdev description of a PCI device includes the attribute Bmanaged='yes', +and the hypervisor driver supports it, then the device is in managed mode, and attempts to use that passthrough device in an active guest will automatically behave as if Bnodedev-detach (guest start, device hot-plug) and Bnodedev-reattach (guest stop, device hot-unplug) were -called at the right points (currently, qemu does this for PCI devices, -but not USB). If a device is not marked as managed, then it must -manually be detached before guests can use it, and manually reattached -to be returned to the host. Also, if a device is manually detached, +called at the right points. If a PCI device is not marked as managed, +then it must manually be detached before guests can use it, and manually +reattached to be returned to the host. Also, if a device is manually detached, then the host does not regain control of the device without a matching reattach, even if the guests use the device in managed mode. @@ -2712,8 +2711,8 @@ Icap and I--tree are mutually exclusive. Declare that Inodedev is no longer in use by any guests, and that the host can resume normal use of the device. This is done -automatically for devices in managed mode, but must be done explicitly -to match any explicit Bnodedev-detach. +automatically for PCI devices in managed mode and USB devices, but +must be done explicitly to match any explicit Bnodedev-detach. =item Bnodedev-reset Inodedev -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] libxl: fix dom0 balloon logic
On Tue, 24 Mar 2015, Jim Fehlig wrote:
Recent testing on large memory systems revealed a bug in the Xen xl
tool's freemem() function. When autoballooning is enabled, freemem()
is used to ensure enough memory is available to start a domain,
ballooning dom0 if necessary. When ballooning large amounts of memory
from dom0, freemem() would exceed its self-imposed wait time and
return an error. Meanwhile, dom0 continued to balloon. Starting the
domain later, after sufficient memory was ballooned from dom0, would
succeed. The libvirt implementation in libxlDomainFreeMem() suffers
the same bug since it is modeled after freemem().
In the end, the best place to fix the bug on the Xen side was to
slightly change the behavior of libxl_wait_for_memory_target().
Instead of failing after caller-provided wait_sec, the function now
blocks as long as dom0 memory ballooning is progressing. It will return
failure only when more memory is needed to reach the target and wait_sec
have expired with no progress being made. See xen.git commit fd3aa246.
There was a dicussion on how this would affect other libxl apps like
libvirt
http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html
If libvirt containing this patch was build against a Xen containing
the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem()
will fail after 30 sec and domain creation will be terminated.
Without this patch and with old libxl_wait_for_memory_target() behavior,
libxlDomainFreeMem() does not succeed after 30 sec, but returns success
anyway. Domain creation continues resulting in all sorts of fun stuff
like cpu soft lockups in the guest OS. It was decided to properly fix
libxl_wait_for_memory_target(), and if anything improve the default
behavior of apps using the freemem reference impl in xl.
xl was patched to accommodate the change in libxl_wait_for_memory_target()
with xen.git commit 883b30a0. This patch does the same in the libxl
driver. While at it, I changed the logic to essentially match
freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner
IMO and will make it easier to spot future, potentially interesting
divergences.
Signed-off-by: Jim Fehlig jfeh...@suse.com
Reviewed-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
V2: Actually use libxl_wait_for_memory_target(), instead of
libxl_wait_for_free_memory()
src/libxl/libxl_domain.c | 55
+++-
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 407a9bd..a1739aa 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1121,38 +1121,39 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv,
libxl_domain_config *d_config)
{
uint32_t needed_mem;
uint32_t free_mem;
-size_t i;
-int ret = -1;
+int ret;
int tries = 3;
int wait_secs = 10;
-if ((ret = libxl_domain_need_memory(priv-ctx, d_config-b_info,
-needed_mem)) = 0) {
-for (i = 0; i tries; ++i) {
-if ((ret = libxl_get_free_memory(priv-ctx, free_mem)) 0)
-break;
+ret = libxl_domain_need_memory(priv-ctx, d_config-b_info,
needed_mem);
+if (ret 0)
+goto error;
-if (free_mem = needed_mem) {
-ret = 0;
-break;
-}
+do {
+ret = libxl_get_free_memory(priv-ctx, free_mem);
+if (ret 0)
+goto error;
-if ((ret = libxl_set_memory_target(priv-ctx, 0,
- free_mem - needed_mem,
- /* relative */ 1, 0)) 0)
-break;
+if (free_mem = needed_mem)
+return 0;
-ret = libxl_wait_for_free_memory(priv-ctx, 0, needed_mem,
- wait_secs);
-if (ret == 0 || ret != ERROR_NOMEM)
-break;
+ret = libxl_set_memory_target(priv-ctx, 0,
+ free_mem - needed_mem,
+ /* relative */ 1, 0);
+if (ret 0)
+goto error;
-if ((ret = libxl_wait_for_memory_target(priv-ctx, 0, 1)) 0)
-break;
-}
-}
+ret = libxl_wait_for_memory_target(priv-ctx, 0, wait_secs);
+if (ret 0)
+goto error;
-return ret;
+tries--;
+} while (tries 0);
+
+ error:
+virReportSystemError(ret, %s,
+ _(Failed to balloon domain0 memory));
+return -1;
}
static void
@@ -1271,12 +1272,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
priv-ctx, d_config) 0)
goto endjob;
-if (cfg-autoballoon
