[libvirt] [PATCH] docs: Fix simple typo s/ a API/ an API/
Signed-off-by: Martin Kletzander --- Notes: Pushed as trivial docs/internals/rpc.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/internals/rpc.html.in b/docs/internals/rpc.html.in index 627d7aa..f2ed64f 100644 --- a/docs/internals/rpc.html.in +++ b/docs/internals/rpc.html.in @@ -597,7 +597,7 @@ Example with buck passing - In the first example, a second thread issues a API call + In the first example, a second thread issues an API call while the first thread holds the buck. The reply to the first call arrives first, so the buck is passed to the second thread. -- 2.2.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: migration: Unlock vm if ACL check in Perform phase of protocol v2 fails
On 12/08/2014 07:31 PM, Peter Krempa wrote: > Avoid leaving the domain locked on a failed ACL check. Introduced in > commit abf75aea247e (Add ACL checks into the QEMU driver). > --- > src/qemu/qemu_driver.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] define NTF_{SELF,MASTER} if undefined
Older kernel headers lack this definition (e.g. Debian Wheezy's 3.2) --- src/util/virnetdevbridge.c | 9 + 1 file changed, 9 insertions(+) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 73ec40b..d92a9de 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -919,6 +919,15 @@ virNetDevBridgeSetVlanFiltering(const char *brname ATTRIBUTE_UNUSED, #if defined(__linux__) && defined(HAVE_LIBNL) + +#ifndef NTF_SELF +#define NTF_SELF 0x02 +#endif + +#ifndef NTF_MASTER +#define NTF_MASTER 0x04 +#endif + /* virNetDevBridgeFDBAddDel: * @mac: the MAC address being added to the table * @ifname: name of the port (interface) of the bridge that wants this MAC -- 2.1.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2]qemu: output error when try to hotplug/coldplug a unsupported device
When use attach-device to hotplug a qemu unsupported console, command will success and add a XML to the running guest, but donnot do anything in qemu side. Add a check in qemuBuildConsoleChrDeviceStr, and output a error when try to attach a qemu unsupport console. About report error for qemu unsupported Chr device when cold-plug, I think this maybe unnessary in this place, because we will check it when we start the guest and it will report a clear error.But if we use qemu* header func add a qemu unsupported things to qemu guest, it seems strange. Luyao Huang (2): qemu: output error when try to hotplug unsupport console qemu: add a check when cold-plug a Chr device src/qemu/qemu_command.c | 6 - src/qemu/qemu_hotplug.c | 64 + 2 files changed, 69 insertions(+), 1 deletion(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] qemu: output error when try to hotplug unsupport console
https://bugzilla.redhat.com/show_bug.cgi?id=1164627
When try to use attach-device to hotplug a qemu
unsupport sonsole, command will return success,
and add a console in vm's xml.But this doesn't work
in qemu, qemu doesn't support these console.Add
a error output when try to hotplug a unsupport console
in qemuBuildConsoleChrDeviceStr.
Signed-off-by: Luyao Huang
---
src/qemu/qemu_command.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1399ce4..7dced5f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10069,13 +10069,17 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr,
break;
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL:
+break;
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE:
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN:
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML:
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC:
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ:
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST:
-break;
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported console target type %s"),
+
NULLSTR(virDomainChrConsoleTargetTypeToString(chr->targetType)));
+return ret;
}
ret = 0;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] qemu: add a check when cold-plug a Chr device
Add a func just check the base target type which
qemu support. But i still doubt this will be useful
, we already have a check when we try to start the
vm. And this check only check the target type, and
the other things will be done in virDomainChrDefParseXML.
Signed-off-by: Luyao Huang
---
src/qemu/qemu_hotplug.c | 64 +
1 file changed, 64 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b9a0cee..fe91ec7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1384,10 +1384,74 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr
driver,
}
+static int
+qemuDomainChrCheckDefSupport(virDomainChrDefPtr chr)
+{
+int ret = -1;
+
+switch (chr->deviceType) {
+case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
+switch ((virDomainChrSerialTargetType) chr->targetType) {
+case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
+case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
+ret = 0;
+break;
+
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported serial target type %s for qemu"),
+
NULLSTR(virDomainChrSerialTargetTypeToString(chr->targetType)));
+break;
+}
+break;
+
+case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
+ret = 0;
+break;
+
+case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
+switch ((virDomainChrChannelTargetType) chr->targetType) {
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
+ret = 0;
+break;
+
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported channel target type %s for qemu"),
+
NULLSTR(virDomainChrChannelTargetTypeToString(chr->targetType)));
+break;
+}
+break;
+
+case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
+switch ((virDomainChrConsoleTargetType) chr->targetType) {
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
+ret = 0;
+break;
+
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported console target type %s for qemu"),
+
NULLSTR(virDomainChrConsoleTargetTypeToString(chr->targetType)));
+break;
+}
+break;
+}
+
+return ret;
+}
+
int
qemuDomainChrInsert(virDomainDefPtr vmdef,
virDomainChrDefPtr chr)
{
+if (qemuDomainChrCheckDefSupport(chr) < 0)
+return -1;
+
if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl-Sys-Virt][PATCH] Fix memory corruption when calling migrate_to_uri
The variable `nparams` didn't change accordingly when adding a new parameter in commit b2cecf73. This patch fixes https://bugzilla.redhat.com/show_bug.cgi?id=1171938 Signed-off-by: Hao Liu --- Virt.xs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Virt.xs b/Virt.xs index 763a117..688b0d7 100644 --- a/Virt.xs +++ b/Virt.xs @@ -4342,7 +4342,7 @@ _migrate_to_uri(dom, desturi, newparams, flags=0) virTypedParameter *params; int nparams; PPCODE: - nparams = 5; + nparams = 6; Newx(params, nparams, virTypedParameter); strncpy(params[0].field, VIR_MIGRATE_PARAM_URI, -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] virkeycode: add virKeynameFromKeycode function
Add virKeynameFromKeycode for later xen/libxl sendkey usage.
Signed-off-by: Chunyan Liu
---
src/libvirt_private.syms | 1 +
src/util/virkeycode.c| 17 +
src/util/virkeycode.h| 1 +
3 files changed, 19 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6df2784..087b75f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1540,6 +1540,7 @@ virKeycodeSetTypeFromString;
virKeycodeSetTypeToString;
virKeycodeValueFromString;
virKeycodeValueTranslate;
+virKeynameFromKeycode;
# util/virkeyfile.h
diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
index 7880a0a..c6b3b36 100644
--- a/src/util/virkeycode.c
+++ b/src/util/virkeycode.c
@@ -124,3 +124,20 @@ int virKeycodeValueTranslate(virKeycodeSet from_codeset,
return -1;
}
+
+const char *
+virKeynameFromKeycode(virKeycodeSet codeset, int keycode)
+{
+size_t i;
+
+for (i = 0; i < VIR_KEYMAP_ENTRY_MAX; i++) {
+if (!virKeymapNames[codeset] ||
+!virKeymapValues[codeset])
+continue;
+
+if (virKeymapValues[codeset][i] == keycode)
+return virKeymapNames[codeset][i];
+}
+
+return NULL;
+}
diff --git a/src/util/virkeycode.h b/src/util/virkeycode.h
index 6947cfe..aefb1c9 100644
--- a/src/util/virkeycode.h
+++ b/src/util/virkeycode.h
@@ -29,5 +29,6 @@ int virKeycodeValueFromString(virKeycodeSet codeset, const
char *keyname);
int virKeycodeValueTranslate(virKeycodeSet from_codeset,
virKeycodeSet to_offset,
int key_value);
+const char *virKeynameFromKeycode(virKeycodeSet codeset, int keycode);
#endif
--
1.8.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] libxl: add .domainSendKey
libxl supports sysrq. Add .domainSendKey function to support
sending sysrq key.
Signed-off-by: Chunyan Liu
---
src/libxl/libxl_driver.c | 89
1 file changed, 89 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 53c87ce..6cc5fe6 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -56,6 +56,7 @@
#include "viratomic.h"
#include "virhostdev.h"
#include "network/bridge_driver.h"
+#include "virkeycode.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -4729,6 +4730,93 @@ libxlDomainMigrateConfirm3Params(virDomainPtr domain,
return libxlDomainMigrationConfirm(driver, vm, flags, cancelled);
}
+typedef struct keyname_to_letter {
+ const char *keyname;
+ const char *letter;
+} keyname_to_letter;
+
+static const keyname_to_letter sysrq_keymap[] = {
+{"KEY_0", "0"}, {"KEY_1", "1"}, {"KEY_2", "2"}, {"KEY_3", "3"},
+{"KEY_4", "4"}, {"KEY_5", "5"}, {"KEY_6", "6"}, {"KEY_7", "7"},
+{"KEY_8", "8"}, {"KEY_9", "9"}, {"KEY_A", "a"}, {"KEY_B", "b"},
+{"KEY_C", "c"}, {"KEY_D", "d"}, {"KEY_E", "e"}, {"KEY_F", "f"},
+{"KEY_G", "g"}, {"KEY_H", "h"}, {"KEY_I", "i"}, {"KEY_J", "j"},
+{"KEY_K", "k"}, {"KEY_L", "l"}, {"KEY_M", "m"}, {"KEY_N", "n"},
+{"KEY_O", "o"}, {"KEY_P", "p"}, {"KEY_Q", "q"}, {"KEY_R", "r"},
+{"KEY_S", "s"}, {"KEY_T", "t"}, {"KEY_U", "u"}, {"KEY_V", "v"},
+{"KEY_W", "w"}, {"KEY_X", "x"}, {"KEY_Y", "y"}, {"KEY_Z", "z"},
+{NULL, NULL}
+};
+
+static int
+libxlDomainSendKey(virDomainPtr dom,
+ unsigned int codeset,
+ unsigned int holdtime ATTRIBUTE_UNUSED,
+ unsigned int *keycodes,
+ int nkeycodes,
+ unsigned int flags)
+{
+virDomainObjPtr vm;
+libxlDomainObjPrivatePtr priv;
+const char *keyname0, *keyname1;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(vm = libxlDomObjFromDomain(dom)))
+goto cleanup;
+
+priv = vm->privateData;
+
+if (virDomainSendKeyEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+/* check key, only support sending magic sysrq. Like:
+ * #virsh send-key guest1 KEY_LEFTALT KEY_SYSRQ KEY_H
+ */
+if (nkeycodes < 3)
+goto non_sysrq;
+
+keyname0 = virKeynameFromKeycode(codeset, keycodes[0]);
+keyname1 = virKeynameFromKeycode(codeset, keycodes[1]);
+if (!keyname0 || !keyname1)
+goto non_sysrq;
+
+if ((STREQ(keyname0, "KEY_LEFTALT") && STREQ(keyname1, "KEY_SYSRQ")) ||
+(STREQ(keyname1, "KEY_LEFTALT") && STREQ(keyname0, "KEY_SYSRQ"))) {
+const keyname_to_letter *item = sysrq_keymap;
+char *key = NULL;
+const char *keyname = virKeynameFromKeycode(codeset, keycodes[2]);
+
+while (item && item->keyname) {
+if (keyname && STREQ(item->keyname, keyname)) {
+if (VIR_STRDUP(key, item->letter) < 0)
+goto cleanup;
+break;
+}
+item++;
+}
+
+if (!key)
+goto non_sysrq;
+
+ret = libxl_send_sysrq(priv->ctx, vm->def->id, key[0]);
+VIR_FREE(key);
+goto cleanup;
+
+} else {
+goto non_sysrq;
+}
+
+ non_sysrq:
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ "%s", _("Sending non-sysrq keys is not supported"));
+
+ cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
static virHypervisorDriver libxlDriver = {
.no = VIR_DRV_LIBXL,
@@ -4824,6 +4912,7 @@ static virHypervisorDriver libxlDriver = {
.domainMigratePerform3Params = libxlDomainMigratePerform3Params, /* 1.2.6
*/
.domainMigrateFinish3Params = libxlDomainMigrateFinish3Params, /* 1.2.6 */
.domainMigrateConfirm3Params = libxlDomainMigrateConfirm3Params, /* 1.2.6
*/
+.domainSendKey = libxlDomainSendKey, /* 1.2.11 */
};
static virStateDriver libxlStateDriver = {
--
1.8.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] support sysrq in xen/libxl driver
xm/xend and libxl already support sending sysrq key. Adding the equivalant to libvirt. Chunyan Liu (3): virkeycode: add virKeynameFromKeycode function xen: add .domainSendKey libxl: add .domainSendKey src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 89 src/util/virkeycode.c| 17 + src/util/virkeycode.h| 1 + src/xen/xen_driver.c | 85 + src/xen/xend_internal.c | 21 src/xen/xend_internal.h | 1 + 7 files changed, 215 insertions(+) -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] xen: add .domainSendKey
xm/xend supports sysrq command. Add .domainSendKey function to support
sending sysrq key.
Signed-off-by: Chunyan Liu
---
src/xen/xen_driver.c| 85 +
src/xen/xend_internal.c | 21
src/xen/xend_internal.h | 1 +
3 files changed, 107 insertions(+)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index c9f4159..434236e 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -67,6 +67,7 @@
#include "configmake.h"
#include "virstring.h"
#include "viraccessapicheck.h"
+#include "virkeycode.h"
#define VIR_FROM_THIS VIR_FROM_XEN
@@ -2738,6 +2739,89 @@ xenUnifiedNodeSuspendForDuration(virConnectPtr conn,
return nodeSuspendForDuration(target, duration, flags);
}
+typedef struct keyname_to_letter {
+ const char *keyname;
+ const char *letter;
+} keyname_to_letter;
+
+static const keyname_to_letter sysrq_keymap[] = {
+{"KEY_0", "0"}, {"KEY_1", "1"}, {"KEY_2", "2"}, {"KEY_3", "3"},
+{"KEY_4", "4"}, {"KEY_5", "5"}, {"KEY_6", "6"}, {"KEY_7", "7"},
+{"KEY_8", "8"}, {"KEY_9", "9"}, {"KEY_A", "a"}, {"KEY_B", "b"},
+{"KEY_C", "c"}, {"KEY_D", "d"}, {"KEY_E", "e"}, {"KEY_F", "f"},
+{"KEY_G", "g"}, {"KEY_H", "h"}, {"KEY_I", "i"}, {"KEY_J", "j"},
+{"KEY_K", "k"}, {"KEY_L", "l"}, {"KEY_M", "m"}, {"KEY_N", "n"},
+{"KEY_O", "o"}, {"KEY_P", "p"}, {"KEY_Q", "q"}, {"KEY_R", "r"},
+{"KEY_S", "s"}, {"KEY_T", "t"}, {"KEY_U", "u"}, {"KEY_V", "v"},
+{"KEY_W", "w"}, {"KEY_X", "x"}, {"KEY_Y", "y"}, {"KEY_Z", "z"},
+{NULL, NULL}
+};
+
+static int
+xenUnifiedDomainSendKey(virDomainPtr dom,
+unsigned int codeset,
+unsigned int holdtime ATTRIBUTE_UNUSED,
+unsigned int *keycodes,
+int nkeycodes,
+unsigned int flags)
+{
+int ret = -1;
+virDomainDefPtr def;
+const char *keyname0, *keyname1;
+
+virCheckFlags(0, -1);
+
+if (!(def = xenGetDomainDefForDom(dom)))
+goto cleanup;
+
+if (virDomainSendKeyEnsureACL(dom->conn, def) < 0)
+goto cleanup;
+
+/* check key, only support sending magic sysrq. Like:
+ * #virsh send-key guest1 KEY_LEFTALT KEY_SYSRQ KEY_H
+ */
+if (nkeycodes < 3)
+goto non_sysrq;
+
+keyname0 = virKeynameFromKeycode(codeset, keycodes[0]);
+keyname1 = virKeynameFromKeycode(codeset, keycodes[1]);
+if (!keyname0 || !keyname1)
+goto non_sysrq;
+
+if ((STREQ(keyname0, "KEY_LEFTALT") && STREQ(keyname1, "KEY_SYSRQ")) ||
+(STREQ(keyname1, "KEY_LEFTALT") && STREQ(keyname0, "KEY_SYSRQ"))) {
+const keyname_to_letter *item = sysrq_keymap;
+char *key = NULL;
+const char *keyname = virKeynameFromKeycode(codeset, keycodes[2]);
+
+while (item && item->keyname) {
+if (keyname && STREQ(item->keyname, keyname)) {
+if (VIR_STRDUP(key, item->letter) < 0)
+goto cleanup;
+break;
+}
+item++;
+}
+
+if (!key)
+goto non_sysrq;
+
+ret = xenDaemonDomainSysrq(dom->conn, def, key);
+VIR_FREE(key);
+goto cleanup;
+
+} else {
+goto non_sysrq;
+}
+
+ non_sysrq:
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ "%s", _("Sending non-sysrq keys is not supported"));
+
+ cleanup:
+virDomainDefFree(def);
+return ret;
+}
/*- Register with libvirt.c, and initialize Xen drivers. -*/
@@ -2836,6 +2920,7 @@ static virHypervisorDriver xenUnifiedDriver = {
.nodeSuspendForDuration = xenUnifiedNodeSuspendForDuration, /* 0.9.8 */
.nodeGetMemoryParameters = xenUnifiedNodeGetMemoryParameters, /* 0.10.2 */
.nodeSetMemoryParameters = xenUnifiedNodeSetMemoryParameters, /* 0.10.2 */
+.domainSendKey = xenUnifiedDomainSendKey, /* 1.2.11 */
};
/**
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index b233b6b..eabb6ed 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1349,6 +1349,27 @@ xenDaemonDomainReboot(virConnectPtr conn,
virDomainDefPtr def)
}
/**
+ * xenDaemonDomainSysrq:
+ * @conn: the connection object
+ * @def: the domain to destroy
+ *
+ * Send a sysrq to a domain.
+ *
+ * Returns 0 in case of success, -1 (with errno) in case of error.
+ */
+int
+xenDaemonDomainSysrq(virConnectPtr conn, virDomainDefPtr def, char *key)
+{
+if (def->id < 0) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _("Domain %s isn't running."), def->name);
+return -1;
+}
+
+return xend_op(conn, def->name, "op", "sysrq", "key", key, NULL);
+}
+
+/**
* xenDaemonDomainDestroy:
* @conn: the connection object
* @def: the domain to destroy
diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
index 814330d..6706394 100644
--- a/src/xen/xend_internal.h
+++ b/src/xen/xend_internal.h
@@ -213,5 +213,6 @@ int xenDaemonSet
[libvirt] Entering freeze for 1.2.11, rc1 available
As planned, I tagged 1.2.11-rc1 in git and made signed tarballs and rpms available at the usual place: ftp://libvirt.org/libvirt/ This seems to work fine in my limited testing, but please give it a serious try ! I understand Peter concerns w.r.t. the parallels patches, IMHO if this doesn't touch common code and is fine by the maintainers, then pushing in time for rc2 is reasonable even if we are past freeze. The plan is to put out an rc2 in a couple of days and get the final release this w.e. unless there is a serious issue blocking it. thanks in advance for testing and reports ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Move check for XML::XPath into bootstrap
On 12/08/2014 04:13 AM, Martin Kletzander wrote: > The module XML::XPath is needed when building from git only (no need to > have it when building from tarball), so this patch moves the check from > specfile into bootstrap.conf. > > Signed-off-by: Martin Kletzander > --- > This patch needs a gnulib update with this proposed patch in it: > https://lists.gnu.org/archive/html/bug-gnulib/2014-12/msg00108.html ACK; the gnulib update is now in place, so this should just work. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Waiting for review of [PATCH v2 2/21] hyperv: implementation of virConnectGetVersion
On 12/08/2014 03:32 AM, Yves Vinter wrote: > No answer ??? > > What does it mean : > > -you don't have a window for the review ? Probably truer than we wish - the biggest bottleneck in open source projects is review time. > > -you are not interested in getting these developments ? To the contrary, we ARE interested. It's just a juggling act between fixing known bugs in other areas and taking the mental time to review relatively unfamiliar code. > > -you don't agree with the delivery process I proposed ? > > -you prefer getting the whole set of the v2 versions before reviewing > ? At this point, a rebased v2 will probably get reviewed faster than waiting for someone to jump on v1. Also, one of the BEST ways to get your code reviewed faster is to return the favor by reviewing other patches. Even if you admit that your review is weak and you'd like a second set of eyes, any review that you can offer to someone else's patches shows that you are serious about contributing to the community, and therefore deserve the community's time to review your patches. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: Correct invalid hyperlinks
On 12/08/2014 04:22 AM, Martin Kletzander wrote: >> How can you add a perl module in "buildreq"? I'm not familiar with how >> bootstrap works. >> > > I had a look at that and posted a patch for gnulib [1] to work with > perl modules and additional path for libvirt [2] which takes that into > account. Let me see if that works for you. > > Martin > > [1] https://lists.gnu.org/archive/html/bug-gnulib/2014-12/msg00108.html > [2] https://www.redhat.com/archives/libvir-list/2014-December/msg00399.html The gnulib update is now in place. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] maint: update to latest gnulib
Several portability changes, but the one we are most interested in
is the improvement to bootstrap to detect perl modules.
This patch doesn't actually change our bootstrap requirements
(that will be a separate patch), but sets the stage for it.
* .gnulib: Update to latest, for bootstrap improvements.
* bootstrap: Regenerate from upstream.
Signed-off-by: Eric Blake
---
Upstream gnulib is still working out an issue with modern mingw
supporting %lld in printf by default; until that is sorted, we do
not want to pick up gnulib cf88e56 in isolation.
Pushing under the gnulib maintenance rule, before we freeze.
* .gnulib 9565c3b...3914f31 (67):
> bootstrap: Allow perl modules in $buildreq
> apply _GL_ATTRIBUTE_PURE to some inline functions
> vasnprintf: fix potential incorrect errno
> vasnprintf: fix potential use after free
> autoupdate
> filevercmp, posixtm: avoid compiler warnings with -O3
> Fix LDBL80_WORDS macro on big endian platforms.
> autoupdate
> git-version-gen: do not print new line characters
> gnulib-tool: recognize x:* as an absolute path
> argp: avoid extraneous translation and mem leak with empty pre doc
> autoupdate
> doc: mention that _BSD_SOURCE is deprecated for _DEFAULT_SOURCE
> uniname/uniname-tests: skip if system's libunistring is used
> printf: fix configure check on big endian systems
> pipe-filter-gi, pipe-filter-ii: port to AIX
> gitlog-to-changelog: add --until
> update from texinfo
> extern-inline: update commentary about GCC bugs
> gen-uni-tables: untabify
> gen-uni-tables: check out-of-range values added to 3-level tables
> gen-uni-tables: utilize 'assert'
> gen-uni-tables: cosmetic improvements
> fcntl-h-tests: port to PA-RISC GNU/Linux
> fts: port to C89
> unistd: port to iOS
> obstack: do not reject malloc-style obstack_chunkfun, obstack_freefun
> autoupdate
> update from texinfo
> obstack: avoid potentially-nonportable function casts
> obstack: fix macro return values
> obstack: do not assume system-supplied obstack is size_t safe
> obstack: port to platforms that #define __alignof__
> linkat: don't unconditionally replace on GNU/Linux
> linkat: wrap to handle symlinks on OS X 10.10
> open, openat: document nonstandard FreeBSD, NetBSD O_NOFOLLOW errno
> obstack: add NEWS entry for recent incompatible changes
> mountlist: don't use libmount to decide on dummy/remote
> maint: add missing ChangeLog entries for Modra's obstack changes
> obstack: prefer __alignof__ to alignof
> obstack: prefer alignof to calculating alignments by hand
> obstack: use size_t alignments and check for overflow
> obstack: 64-bit obstack support, part 3
> obstack: 64-bit obstack support, part 2
> obstack: 64-bit obstack support, part 1
> obstack: tidy part 2
> obstack: tidy part 1
> socketlib, sockets, sys_socket: Use AC_REQUIRE to pacify autoconf.
> iconv: avoid false detection of non-working iconv
> bootstrap: print more diagnostics for missing programs
> bootstrap: only update the gnulib submodule
> symlinkat: port to AIX 7.1
> readlinkat: port to AIX 7.1
> remove spurious {
> modules/fcntl: fix error reporting by dupfd
> basename, dirname: Improve documentation.
> exclude: declare exclude_patopts static
> autoupdate
> dirname: support compilation with C++
> qsort_r: include
> avltree-list: avoid compiler warnings
> qsort_r: new module, for GNU-style qsort_r
> strerror_r-posix: support compilation with C++
> fcntl-h: fix compilation with Intel C++ compiler
> autoupdate
> mountlist: use /proc/self/mountinfo when available
> users.txt: add cmogstored
---
.gnulib | 2 +-
bootstrap | 42 ++
2 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/.gnulib b/.gnulib
index 9565c3b..3914f31 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 9565c3be73eb6d76b7b42a21d68d2e00a62abb6d
+Subproject commit 3914f3153576e9a5ba4002bde27de05211b5a79c
diff --git a/bootstrap b/bootstrap
index ce90bc4..e0c4ec2 100755
--- a/bootstrap
+++ b/bootstrap
@@ -1,6 +1,6 @@
#! /bin/sh
# Print a version string.
-scriptversion=2013-12-05.23; # UTC
+scriptversion=2014-12-08.12; # UTC
# Bootstrap this package from checked-out sources.
@@ -42,6 +42,9 @@ export LC_ALL
local_gl_dir=gl
+# Honour $PERL, but work even if there is none
+PERL="${PERL-perl}"
+
me=$0
usage() {
@@ -210,7 +213,17 @@ bootstrap_sync=false
use_git=true
check_exists() {
- ($1 --version /dev/null 2>&1
+ if test "$1" = "--verbose"; then
+($2 --version /dev/null 2>&1
+if test $? -ge 126; then
+ # If not found, run with diagnostics as one may be
+ # presented with env variables to set to find the right version
+ ($2 --version /dev/null 2>&1
+ fi
+
test $? -lt 126
}
@@ -408,7 +421,7 @@ sort_ver() { # sort -V is not generally available
get_version() {
app=$1
- $app --version >/dev/null 2>&1 || return 1
+ $app --version
Re: [libvirt] [PATCH v3 2/2] qemu: Automaticly create tap device for VIR_DOMAIN_NET_TYPE_ETHERNET
On 12/8/14, 2:10 AM, "Daniel P. Berrange" wrote:
>On Fri, Dec 05, 2014 at 11:37:05PM +, Anirban Chakraborty wrote:
>>
>>
>> On 12/5/14, 10:43 AM, "Laine Stump" wrote:
>>
>> >On 12/05/2014 06:12 AM, Michal Privoznik wrote:
>> >> @@ -7374,7 +7399,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr
>>cmd,
>> >> }
>> >>
>> >> if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
>> >> -actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
>> >> +actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
>> >> +actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
>> >> tapfdSize = net->driver.virtio.queues;
>> >> if (!tapfdSize)
>> >> tapfdSize = 1;
>> >
>> >So this patch causes libvirt to *always* create a tap device in the
>> >traditional manner for type='ethernet'. I wonder if we can safely do
>> >this without unknowingly breaking some strange functionality. In
>> >particular, what if someone is using type='ethernet' and a script to
>> >create some *other* kind of device that outwardly appears to be a tap
>> >device, but is created in a different manner - currently you can do
>>this
>> >by using type='ethernet' and calling your strange "create my
>> >Franken-tap" command from the script. Once this patch is in, you'll no
>> >longer be able to do this.
>> >
>> >(As a matter of fact, I'm getting some sort of phantom memory about
>> >someone doing that for some different kind of virtual switch, or
>> >possibly some piece of hardware. I can't remember the details though,
>> >and it's possible that I'm mistaken - maybe they *were* just creating a
>> >standard tap device, but then doing strange things to it.)
>>
>> In Open Contrail (www.opencontrail.org), we use this feature where tap
>> interface is created first, so that we know the name of the tap device a
>> priori, before creating the vm. So, this patch will break existing code.
>
>Do you actally pre-create the TAP device though, or merely ask for a
>particular choice of name. AFAIK, then using type=ethernet, QEMU
>will always attempt to create the TAP device itself, honouring any
>name given. Basically whatever QEMU does for type=ethernet, libvirt
>should copy in a 100% functionally equivalent manner. We simply want
>to move the functionality up a level. So there should be no regressions
>if done correctly.
It is created as part of openstack nova vif creation process. Nova libvirt
driver passes in the vif name to the network provider (in this case,
contrail), which in turn uses it internally for various configuration
purposes. What you are saying is that passing a preassigned name to
libvirt for tap device creation would work. In that case, it should be
fine.
Anirban
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: fix mingw printing of pid
Commit c75425734 introduced a compilation failure:
../../src/access/viraccessdriverpolkit.c: In function
'virAccessDriverPolkitCheck':
../../src/access/viraccessdriverpolkit.c:137:5: error: format '%d' expects
argument of type 'int', but argument 9 has type 'pid_t' [-Werror=format=]
VIR_DEBUG("Check action '%s' for process '%d' time %lld uid %d",
^
Since mingw pid_t is 64 bits, it's easier to just follow what we've
done elsewhere and cast to a large enough type when printing pids.
* src/access/viraccessdriverpolkit.c (virAccessDriverPolkitCheck):
Add cast.
Signed-off-by: Eric Blake
---
Pushing under the build-breaker rule.
src/access/viraccessdriverpolkit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/access/viraccessdriverpolkit.c
b/src/access/viraccessdriverpolkit.c
index 3136be7..89bc890 100644
--- a/src/access/viraccessdriverpolkit.c
+++ b/src/access/viraccessdriverpolkit.c
@@ -1,5 +1,5 @@
/*
- * viraccessdriverpolkit.c: polkited access control driver
+ * viraccessdriverpolkit.c: polkitd access control driver
*
* Copyright (C) 2012, 2014 Red Hat, Inc.
*
@@ -134,8 +134,8 @@ virAccessDriverPolkitCheck(virAccessManagerPtr manager
ATTRIBUTE_UNUSED,
&uid) < 0)
goto cleanup;
-VIR_DEBUG("Check action '%s' for process '%d' time %lld uid %d",
- actionid, pid, startTime, uid);
+VIR_DEBUG("Check action '%s' for process '%lld' time %lld uid %d",
+ actionid, (long long) pid, startTime, uid);
rv = virPolkitCheckAuth(actionid,
pid,
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/2] qemu: Automaticly create tap device for VIR_DOMAIN_NET_TYPE_ETHERNET
2014-12-08 23:51 GMT+03:00 Anirban Chakraborty : > > Yes, that will work. So as i see two different point of view... I want to get this patch in next libvirt release What i need to do? -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru jabber: v...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: fix unused variable in mingw
util/virnetdevbridge.c: In function 'virNetDevBridgePortSetLearning':
util/virnetdevbridge.c:359:38: error: unused parameter 'enable'
[-Werror=unused-parameter]
bool enable)
^
* src/util/virnetdevbridge.c (virNetDevBridgePortSetLearning): Mark
unused variable.
Signed-off-by: Eric Blake
---
Pushing under the build-breaker rule.
src/util/virnetdevbridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index 78c4a25..73ec40b 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -356,7 +356,7 @@ virNetDevBridgePortGetLearning(const char *brname
ATTRIBUTE_UNUSED,
int
virNetDevBridgePortSetLearning(const char *brname ATTRIBUTE_UNUSED,
const char *ifname ATTRIBUTE_UNUSED,
- bool enable)
+ bool enable ATTRIBUTE_UNUSED)
{
virReportSystemError(ENOSYS, "%s",
_("Unable to set bridge port learning on this
platform"));
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 3/9] conf: new network bridge device attribute macTableManager
On 12/08/2014 11:33 AM, Daniel P. Berrange wrote: > ACK, design looks good now. Thanks! Based on the previous ACKs of the code, and this ACK of the new name, I've pushed the series. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/2] qemu: Automaticly create tap device for VIR_DOMAIN_NET_TYPE_ETHERNET
On 12/6/14, 2:51 AM, "Vasiliy Tolstov" wrote: >2014-12-06 2:37 GMT+03:00 Anirban Chakraborty : >> In Open Contrail (www.opencontrail.org), we use this feature where tap >> interface is created first, so that we know the name of the tap device a >> priori, before creating the vm. So, this patch will break existing code. > > >May be we can add some flag like managed true/false ? and create tap >only if managed ? Yes, that will work. Anirban -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RESEND PATCH] network: don't allow multiple dhcp sections
On 12/04/2014 04:07 PM, Kyle DeFrancia wrote:
> This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=907779
>
> A element can exist in only one IPv4 address and one IPv6
> address per network. This patch enforces that in virNetworkUpdate.
> ---
> Rebased to latest master, hopefully this works better.
It took awhile to figure out the problem - whatever method you used to
send this patch wrapped long lines and combined them together. I had to
break up and re-format the places I've indicated below with ^^^. Aside
from that I just combined the multiple lines of the for() (because
combined they were still below the 80 character limit) and added another
blank line before and after the new function.
ACK and pushed. Thanks for the bugfix!
(Next time it would save some trouble if you directly used "git
send-email" to sent patches)
> My original message:
> https://www.redhat.com/archives/libvir-list/2014-November/msg00989.html
>
> src/conf/network_conf.c | 31 +++
> 1 file changed, 31 insertions(+)
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 97719ed..92aa9d5 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -3480,6 +3480,31 @@ virNetworkIpDefByIndex(virNetworkDefPtr def, int
> parentIndex) }
^^
> static int
> +virNetworkDefUpdateCheckMultiDHCP(virNetworkDefPtr def,
> + virNetworkIpDefPtr ipdef)
> +{
> +int family = VIR_SOCKET_ADDR_FAMILY(&ipdef->address);
> +size_t i;
> +virNetworkIpDefPtr ip;
> +
> +for (i = 0;
> + (ip = virNetworkDefGetIpByIndex(def, family, i));
> + i++) {
> +if (ip != ipdef) {
> +if (ip->nranges || ip->nhosts) {
> +virReportError(VIR_ERR_OPERATION_INVALID,
> + _("dhcp is supported only for a "
> + "single %s address on each network"),
> + (family == AF_INET) ? "IPv4" : "IPv6");
> +return -1;
> +}
> +}
> +}
> +
> +return 0;
> +}
> +
> +static int
> virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def,
>unsigned int command,
>int parentIndex,
> @@ -3544,6 +3569,9 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr
> def, } else if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
^^^
> (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
>
> +if (virNetworkDefUpdateCheckMultiDHCP(def, ipdef) < 0)
> +goto cleanup;
> +
> /* log error if an entry with same name/address/ip already
> exists */ for (i = 0; i < ipdef->nhosts; i++) {
> if ((host.mac &&
> @@ -3651,6 +3679,9 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr
> def, if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
^^
> (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
>
> +if (virNetworkDefUpdateCheckMultiDHCP(def, ipdef) < 0)
> +goto cleanup;
> +
> if (i < ipdef->nranges) {
> char *startip = virSocketAddrFormat(&range.start);
> char *endip = virSocketAddrFormat(&range.end);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/12] qemu: let blockinfo reuse virStorageSource
On 12/08/2014 06:03 AM, Peter Krempa wrote:
> On 12/06/14 09:14, Eric Blake wrote:
>> Right now, grabbing blockinfo always calls stat on the disk, then
>> opens the image to determine the capacity, using a throw-away
>> virStorageSourcePtr. This has a couple of drawbacks:
>>
>> + * For read-only disks, nothing should be changing unless the user
>> + * has requested a block-commit action. For read-write disks, we
>> + * know some special cases: capacity should not change without a
>> + * block-resize (where capacity is the only stat that requires
>> + * opening a file, and even then, only for non-raw files); and
>> + * physical size of a raw image or of a block device should
>> + * likewise not be changing without block-resize. On the other
>> + * hand, allocation of a raw file can change (if the file is
>> + * sparse, but the amount of sparseness changes due to writes or
>> + * punching holes), and physical size of a non-raw file can
>> + * change.
>
> For a live VM we should grab all of the above directly from the monitor
> and not ever touch the files on the disk. We do that already for the
> bulk stats and for getting the right size when doing storage migration.
Agreed. On my next spin, I'll see about getting block info for live vms
done right. (But first, I'm still in the middle of building a scratch
image with this series, if only so that we have an easier binary to play
with for integration and testing purposes).
>
> This function unfortunately is legacy code compared to the stuff I've
> pointed out
Yes, blockinfo is quite old, so here's hoping I don't break any
semantics in touching it.
>> +++ b/src/util/virstoragefile.h
>> @@ -257,8 +257,9 @@ struct _virStorageSource {
>>
>> virStoragePermsPtr perms;
>> virStorageTimestampsPtr timestamps;
>> -unsigned long long allocation; /* in bytes, 0 if unknown */
>
> Spurious move?
>
>> unsigned long long capacity; /* in bytes, 0 if unknown */
>> +unsigned long long allocation; /* in bytes, 0 if unknown */
>> +unsigned long long physical; /* in bytes, 0 if unknown */
You know, Adam asked the same question last time :)
https://www.redhat.com/archives/libvir-list/2014-November/msg00599.html
It's intentional, to match the order in the public header for block
info. But I'll update the commit message to make it obvious.
>> size_t nseclabels;
>> virSecurityDeviceLabelDefPtr *seclabels;
>>
>
> Also an addition to virStorageSourceCopy is missing.
>
> ACK with the tweak to virStorageSourceCopy
I'll probably wait on this patch until I see how the refactoring goes
for splitting offline vs. online stat gathering.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Set path to console on domain startup.
Anthony PERARD wrote:
> The path to the pty of a Xen PV console is set only in
> virDomainOpenConsole. But this is done too late. A call to
> virDomainGetXMLDesc done before OpenConsole will not have the path to
> the pty, but a call after OpenConsole will.
>
Hi Anthony,
Thanks for the patch. Can you address the comments made by others, my
comments below, and provide a V2?
> e.g. of the current issue.
> Starting a domain with ''
> Then:
> virDomainGetXMLDesc():
>
>
>
>
>
> virDomainOpenConsole()
> virDomainGetXMLDesc():
>
>
>
>
>
>
>
> The patch intend to get the tty path on the first call of GetXMLDesc.
>
> Signed-off-by: Anthony PERARD
> ---
> src/libxl/libxl_domain.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 9c62291..de56054 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -1290,6 +1290,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
> virDomainObjPtr vm,
> if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> goto cleanup_dom;
>
> +if (vm->def->nconsoles) {
> +virDomainChrDefPtr chr = NULL;
> +chr = vm->def->consoles[0];
> +if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
> +libxl_console_type console_type;
> +char *console = NULL;
> +console_type =
> +(chr->targetType ==
> VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
> + LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
> +ret = libxl_console_get_tty(priv->ctx, vm->def->id,
> chr->target.port,
> +console_type, &console);
> +if (!ret)
> +ignore_value(VIR_STRDUP(chr->source.data.file.path,
> console));
>
VIR_STRDUP will not free an existing value. You should VIR_FREE first,
which btw can handle a NULL argument. And since you're initializing
source.data.file.path when starting the domain, I think we can drop the
similar code in libxlDomainOpenConsole().
Regards,
Jim
> +VIR_FREE(console);
> +}
> +}
> +
> if (!start_paused) {
> libxl_domain_unpause(priv->ctx, domid);
> virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
> VIR_DOMAIN_RUNNING_BOOTED);
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Set path to console on domain startup
Anthony PERARD wrote: > Hi, > > I'm trying to fix an issue when using OpenStack with libvirt+xen > (libxenlight). > OpenStack cannot access the console output of a Xen PV guest, because the XML > generated by libvirt for a domain is missing the path to the pty. The path > actually appear in the XML once one call virDomainOpenConsole(). > > The patch intend to get the path to the pty without having to call > virDomainOpenConsole, so I've done the work in libxlDomainStart(). So I have a > few question: > Is libxlDomainStart will be called on restore/migrate/reboot? > Yes. > I guest the function libxlDomainOpenConsole() would not need to do the same > work if the console path is settup properly. > Yes, agreed. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: migration: Unlock vm if ACL check in Finish phase of protocol v2 fails
On 12/08/2014 11:53 AM, Peter Krempa wrote: > Avoid leaving the domain locked on a failed ACL check. Introduced in > commit abf75aea247e (Add ACL checks into the QEMU driver). > --- > src/qemu/qemu_driver.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Should this be squashed with your patch to qemuDomainMigratePerform, since they both affect migration and were both introduced by the ACL commit? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/12] getstats: add block.n.path stat
On 12/08/2014 07:17 AM, Peter Krempa wrote: > On 12/06/14 09:14, Eric Blake wrote: >> I'm about to make block stats optionally more complex to cover >> backing chains, where block.count will no longer equal the number >> of for a domain. For these reasons, it is nicer if the >> statistics output includes the source path (for local files). >> This patch doesn't add anything for network disks, although we >> may decide to add that later. >> >> With this patch, I now see the following for the same domain as >> in earlier patches (one qcow2 file, and an empty cdrom drive): >> $ virsh domstats --block foo >> Domain: 'foo' >> block.count=2 >> block.0.name=hda >> block.0.path=/var/lib/libvirt/images/foo.qcow2 >> block.0.allocation=200704 >> block.0.capacity=42949672960 >> block.0.physical=200704 >> block.1.name=hdc > ACK, 2, 6, and 8 now pushed. Of course, the commit message for 8 had to be tweaked a bit (since the output is waiting on the offline stats of patch 7, which in turn are waiting on the split of offline vs. online in the helper to blockinfo). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] qemu: completely rework reference counting
On 12/05/14 15:03, Martin Kletzander wrote: > There is one problem that causes various errors in the daemon. When > domain is waiting for a job, it is unlocked while waiting on the > condition. However, if that domain is for example transient and being > removed in another API (e.g. cancelling incoming migration), it get's > unref'd. If the first call, that was waiting, fails to get the job, it > unref's the domain object, and because it was the last reference, it > causes clearing of the whole domain object. However, when finishing the > call, the domain must be unlocked, but there is no way for the API to > know whether it was cleaned or not (unless there is some ugly temporary > variable, but let's scratch that). > > The root cause is that our APIs don't ref the objects they are using and > all use the implicit reference that the object has when it is in the > domain list. That reference can be removed when the API is waiting for > a job. And because each domain doesn't do its ref'ing, it results in > the ugly checking of the return value of virObjectUnref() that we have > everywhere. > > This patch changes qemuDomObjFromDomain() to ref the domain (using > virDomainObjListFindByUUIDRef()) and adds qemuDomObjEndAPI() which > should be the only function in which the return value of > virObjectUnref() is checked. This makes all reference counting > deterministic and makes the code a bit clearer. > > Signed-off-by: Martin Kletzander > --- > src/qemu/THREADS.txt | 40 ++- > src/qemu/qemu_domain.c| 29 +- > src/qemu/qemu_domain.h| 12 +- > src/qemu/qemu_driver.c| 708 > -- > src/qemu/qemu_migration.c | 108 +++ > src/qemu/qemu_migration.h | 10 +- > src/qemu/qemu_process.c | 87 +++--- > 7 files changed, 359 insertions(+), 635 deletions(-) > > diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt > index 50a0cf9..53a3415 100644 > --- a/src/qemu/THREADS.txt > +++ b/src/qemu/THREADS.txt ... > @@ -109,7 +117,6 @@ To lock the virDomainObjPtr > To acquire the normal job condition > >qemuDomainObjBeginJob() * > -- Increments ref count on virDomainObjPtr > - Waits until the job is compatible with current async job or no >async job is running > - Waits for job.cond condition 'job.active != 0' using virDomainObjPtr > @@ -122,14 +129,12 @@ To acquire the normal job condition >qemuDomainObjEndJob() > - Sets job.active to 0 > - Signals on job.cond condition > -- Decrements ref count on virDomainObjPtr > > > > To acquire the asynchronous job condition > >qemuDomainObjBeginAsyncJob() > -- Increments ref count on virDomainObjPtr Should we mention that having a ref already is pre-requisite for calling Begin*Job? > - Waits until no async job is running > - Waits for job.cond condition 'job.active != 0' using virDomainObjPtr >mutex ... > @@ -179,12 +183,10 @@ To acquire the QEMU monitor lock as part of an > asynchronous job > To keep a domain alive while waiting on a remote command > >qemuDomainObjEnterRemote() Same here ? > -- Increments ref count on virDomainObjPtr > - Releases the virDomainObjPtr lock > >qemuDomainObjExitRemote() > - Acquires the virDomainObjPtr lock > -- Decrements ref count on virDomainObjPtr > > > Design patterns > @@ -195,18 +197,18 @@ Design patterns > > virDomainObjPtr obj; > > - obj = virDomainFindByUUID(driver->domains, dom->uuid); > + obj = qemuDomObjFromDomain(dom); > > ...do work... > > - virDomainObjUnlock(obj); > + qemuDomObjEndAPI(obj); &obj to match the code. > > > * Updating something directly to do with a virDomainObjPtr > > virDomainObjPtr obj; > > - obj = virDomainFindByUUID(driver->domains, dom->uuid); > + obj = qemuDomObjFromDomain(dom); > > qemuDomainObjBeginJob(obj, QEMU_JOB_TYPE); > > @@ -214,18 +216,15 @@ Design patterns > > qemuDomainObjEndJob(obj); > > - virDomainObjUnlock(obj); > - > - > + qemuDomObjEndAPI(obj); &obj ... > > > * Invoking a monitor command on a virDomainObjPtr > > - > virDomainObjPtr obj; > qemuDomainObjPrivatePtr priv; > > - obj = virDomainFindByUUID(driver->domains, dom->uuid); > + obj = qemuDomObjFromDomain(dom); > > qemuDomainObjBeginJob(obj, QEMU_JOB_TYPE); > > @@ -240,8 +239,7 @@ Design patterns > ...do final work... > > qemuDomainObjEndJob(obj); > - virDomainObjUnlock(obj); > - > + qemuDomObjEndAPI(obj); &obj ... > > > * Running asynchronous job > @@ -249,7 +247,7 @@ Design patterns > virDomainObjPtr obj; > qemuDomainObjPrivatePtr priv; > > - obj = virDomainFindByUUID(driver->domains, dom->uuid); > + obj = qemuDomObjFromDomain(dom); > > qemuDomainObjBeginAsyncJob(obj, QEMU_ASYNC_JOB_TYPE); > qemuDomainObjSetAsyncJobMask(obj, allowedJobs); > @@ -281,7 +279,7 @@ Design patterns >
[libvirt] [PATCH] qemu: migration: Unlock vm if ACL check in Finish phase of protocol v2 fails
Avoid leaving the domain locked on a failed ACL check. Introduced in
commit abf75aea247e (Add ACL checks into the QEMU driver).
---
src/qemu/qemu_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8cec372..0a684e1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11447,41 +11447,43 @@ static virDomainPtr
qemuDomainMigrateFinish2(virConnectPtr dconn,
const char *dname,
const char *cookie ATTRIBUTE_UNUSED,
int cookielen ATTRIBUTE_UNUSED,
const char *uri ATTRIBUTE_UNUSED,
unsigned long flags,
int retcode)
{
virQEMUDriverPtr driver = dconn->privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
virCheckFlags(QEMU_MIGRATION_FLAGS, NULL);
vm = virDomainObjListFindByName(driver->domains, dname);
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN,
_("no domain with matching name '%s'"), dname);
goto cleanup;
}
-if (virDomainMigrateFinish2EnsureACL(dconn, vm->def) < 0)
+if (virDomainMigrateFinish2EnsureACL(dconn, vm->def) < 0) {
+virObjectUnlock(vm);
goto cleanup;
+}
/* Do not use cookies in v2 protocol, since the cookie
* length was not sufficiently large, causing failures
* migrating between old & new libvirtd
*/
dom = qemuMigrationFinish(driver, dconn, vm,
NULL, 0, NULL, NULL, /* No cookies */
flags, retcode, false);
cleanup:
return dom;
}
/***
* Migration Protocol Version 3
***/
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Heads-up about the next release
On 12/06/14 02:59, Daniel Veillard wrote: > The plan exposed last month is to push the new release mid month. > I will be unavailable mostly on 15-16 so pondering pushing next week, > which means entering freeze for example Tuesday morning for a final > release toward Fri 12 or Sat 13. We have close to 300 commits already > since 1.2.10 so that sounds about time :-) > > Please raise any issue with going forward with this plan, There's a few patches for the parallels driver that are around for some time. They already got some review from the parallels folks and should be basically ready to push. I wasn't able to do that yet though as I wanted to compile test them before doing so and I didn't have enough time to install the parallels SDK and compile test them. The open question now is whether we deem enough that the patches were tested and ACKed by parallels folks and push them or whether we need to check them before. Anyhow, they are around for quite a long time and I'd feel better if we'd merge them in this release. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] nwfilter: Add support for icmpv6 filtering
Make use of the ebtables functionality to be able to filter certain
parameters of icmpv6 packets. Extend the XML parser for icmpv6 types,
type ranges, codes, and code ranges. Extend the nwfilter documentation,
schema, and test cases.
Being able to filter icmpv6 types and codes helps extending the DHCP
snooper for IPv6 and filtering at least some parameters of IPv6's NDP
(Neighbor Discovery Protocol) packets. However, the filtering will not
be as good as the filtering of ARP packets since we cannot
check on IP addresses in the payload of the NDP packets.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
docs/formatnwfilter.html.in| 20 +++
docs/schemas/nwfilter.rng | 26 +
src/conf/nwfilter_conf.c | 26 +
src/conf/nwfilter_conf.h | 4 ++
src/nwfilter/nwfilter_ebiptables_driver.c | 80 ++
tests/nwfilterxml2firewalldata/ipv6-linux.args | 16 ++
tests/nwfilterxml2firewalldata/ipv6.xml| 38
tests/nwfilterxml2xmlin/ipv6-test.xml | 38
tests/nwfilterxml2xmlout/ipv6-test.xml | 12
9 files changed, 260 insertions(+)
diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in
index 073b852..e403e33 100644
--- a/docs/formatnwfilter.html.in
+++ b/docs/formatnwfilter.html.in
@@ -1197,6 +1197,26 @@
End of range of valid destination ports; requires
protocol
+ type(Since 1.2.11)
+ UINT8
+ ICMPv6 type; requires protocol to be set to
icmpv6
+
+
+ typeend(Since 1.2.11)
+ UINT8
+ ICMPv6 type end of range; requires protocol to be
set to icmpv6
+
+
+ code(Since 1.2.11)
+ UINT8
+ ICMPv6 code; requires protocol to be set to
icmpv6
+
+
+ code(Since 1.2.11)
+ UINT8
+ ICMPv6 code end of range; requires protocol to be
set to icmpv6
+
+
comment (Since 0.8.5)
STRING
text with max. 256 characters
diff --git a/docs/schemas/nwfilter.rng b/docs/schemas/nwfilter.rng
index 2b54fd5..9df39c0 100644
--- a/docs/schemas/nwfilter.rng
+++ b/docs/schemas/nwfilter.rng
@@ -90,6 +90,7 @@
+
@@ -588,6 +589,31 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 317792e..aed82ad 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -1445,6 +1445,26 @@ static const virXMLAttr2Struct ipv6Attributes[] = {
.datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
.dataIdx = offsetof(virNWFilterRuleDef,
p.ipv6HdrFilter.portData.dataDstPortEnd),
},
+{
+.name = "type",
+.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef,
p.ipv6HdrFilter.dataICMPTypeStart),
+},
+{
+.name = "typeend",
+.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef,
p.ipv6HdrFilter.dataICMPTypeEnd),
+},
+{
+.name = "code",
+.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef,
p.ipv6HdrFilter.dataICMPCodeStart),
+},
+{
+.name = "codeend",
+.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
+.dataIdx = offsetof(virNWFilterRuleDef,
p.ipv6HdrFilter.dataICMPCodeEnd),
+},
COMMENT_PROP_IPHDR(ipv6HdrFilter),
{
.name = NULL,
@@ -2219,6 +2239,12 @@ virNWFilterRuleDefFixup(virNWFilterRuleDefPtr rule)
rule->p.ipv6HdrFilter.ipHdr.dataSrcIPAddr);
COPY_NEG_SIGN(rule->p.ipv6HdrFilter.ipHdr.dataDstIPMask,
rule->p.ipv6HdrFilter.ipHdr.dataDstIPAddr);
+COPY_NEG_SIGN(rule->p.ipv6HdrFilter.dataICMPTypeEnd,
+ rule->p.ipv6HdrFilter.dataICMPTypeStart);
+COPY_NEG_SIGN(rule->p.ipv6HdrFilter.dataICMPCodeStart,
+ rule->p.ipv6HdrFilter.dataICMPTypeStart);
+COPY_NEG_SIGN(rule->p.ipv6HdrFilter.dataICMPCodeEnd,
+ rule->p.ipv6HdrFilter.dataICMPTypeStart);
virNWFilterRuleDefFixupIPSet(&rule->p.ipv6HdrFilter.ipHdr);
break;
diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
index f81df60..6e68ecc 100644
--- a/src/conf/nwfilter_conf.h
+++ b/src/conf/nwfilter_conf.h
@@ -265,6 +265,10 @@ struct _ipv6HdrFilterDef {
ethHdrDataDef ethHdr;
ipHdrDataDef ipHdr;
portDataDefportData;
+nwItemDesc dataICMPTypeStart;
+nwItemDesc d
[libvirt] [PATCH] qemu: migration: Unlock vm if ACL check in Perform phase of protocol v2 fails
Avoid leaving the domain locked on a failed ACL check. Introduced in
commit abf75aea247e (Add ACL checks into the QEMU driver).
---
src/qemu/qemu_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ed8e140..8cec372 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11393,51 +11393,53 @@ static int
qemuDomainMigratePerform(virDomainPtr dom,
const char *cookie,
int cookielen,
const char *uri,
unsigned long flags,
const char *dname,
unsigned long resource)
{
virQEMUDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
const char *dconnuri = NULL;
virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
if (virLockManagerPluginUsesState(driver->lockManager)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot use migrate v2 protocol with lock manager
%s"),
virLockManagerPluginGetName(driver->lockManager));
goto cleanup;
}
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
-if (virDomainMigratePerformEnsureACL(dom->conn, vm->def) < 0)
+if (virDomainMigratePerformEnsureACL(dom->conn, vm->def) < 0) {
+virObjectUnlock(vm);
goto cleanup;
+}
if (flags & VIR_MIGRATE_PEER2PEER) {
dconnuri = uri;
uri = NULL;
}
/* Do not output cookies in v2 protocol, since the cookie
* length was not sufficiently large, causing failures
* migrating between old & new libvirtd.
*
* Consume any cookie we were able to decode though
*/
ret = qemuMigrationPerform(driver, dom->conn, vm,
NULL, dconnuri, uri, NULL, NULL,
cookie, cookielen,
NULL, NULL, /* No output cookies in v2 */
flags, dname, resource, false);
cleanup:
return ret;
}
/* Finish is the third and final step, and it runs on the destination host. */
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/12] getstats: prepare for dynamic block.count stat
On 12/08/2014 07:26 AM, Peter Krempa wrote:
> On 12/08/14 15:19, Peter Krempa wrote:
>> On 12/06/14 09:14, Eric Blake wrote:
>>> A coming patch will make it optionally possible to list backing
>>> chain block stats; in this mode of operation, block.counts is no
>>> longer the number of in the domain, but the number of
>>> blocks in the array being reported. We still want block.count
>>> listed first, but rather than iterate the tree twice (once to
>>> count, and once to list stats), it's easier to just touch things
>>> up after the fact.
>>>
>>> * src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Compute count
>>> after the fact.
>>>
>>> Signed-off-by: Eric Blake
>>> ---
>>> src/qemu/qemu_driver.c | 9 -
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>
> Compiler complains:
>
> qemu/qemu_driver.c: In function 'qemuDomainGetStatsBlock':
> qemu/qemu_driver.c:18630:46: error: 'i' may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
> record->params[count_index].value.ui = i;
> ^
You must be compiling at -O2 while I was not :)
Found the culprit: the QEMU_ADD_COUNT_PARAM macro has a hidden goto,
that can bypass the initialization of i=0 in the for loop; the solution
is trivial.
>>
>> ACK,
>>
>
> if you fix the issue above.
I'll be pushing the acked patches that can be reshuffled without
dependencies, and reposting the rest of the series with fixes
incorporated...
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index bb516af..78d1bdd 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -18539,7 +18539,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
int *maxparams,
unsigned int privflags)
{
-size_t i;
+size_t i = 0;
int ret = -1;
int rc;
virHashTablePtr stats = NULL;
@@ -18568,7 +18568,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
count_index = record->nparams;
QEMU_ADD_COUNT_PARAM(record, maxparams, "block", 0);
-for (i = 0; i < dom->def->ndisks; i++) {
+for (i; i < dom->def->ndisks; i++) {
qemuBlockStats *entry;
virDomainDiskDefPtr disk = dom->def->disks[i];
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Set path to console on domain startup
On Mon, Dec 08, 2014 at 04:40:04PM +0100, Ján Tomko wrote: > On 12/05/2014 05:30 PM, Anthony PERARD wrote: > > Hi, > > > > I'm trying to fix an issue when using OpenStack with libvirt+xen > > (libxenlight). > > OpenStack cannot access the console output of a Xen PV guest, because the > > XML > > generated by libvirt for a domain is missing the path to the pty. The path > > actually appear in the XML once one call virDomainOpenConsole(). > > > > The patch intend to get the path to the pty without having to call > > virDomainOpenConsole, so I've done the work in libxlDomainStart(). So I > > have a > > few question: > > Is libxlDomainStart will be called on restore/migrate/reboot? > > I guest the function libxlDomainOpenConsole() would not need to do the same > > work if the console path is settup properly. > > > > There is a bug report about this: > > https://bugzilla.redhat.com/show_bug.cgi?id=1170743 > > > > Hi, > > you can leave the bugzilla link in the commit message, if somebody ever needs > more context. Ok. > (And the patch looks good to me, but I'm no libxl expert) Thanks, -- Anthony PERARD -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/12] getstats: avoid memory leak on OOM
On 12/08/2014 03:50 AM, Peter Krempa wrote:
> On 12/06/14 09:14, Eric Blake wrote:
>> qemuDomainGetStatsBlock() could leak a stats hash table if it
>> encountered OOM while populating the virTypedParameters.
>> Oddly, the fix doesn't even touch qemuDomainGetStatsBlock :)
>> + cleanup:
>
> With this change qemuDomainGetStatsInterface will return success even in
> case when the OOM occurred. You need to add a 'ret' variable set to -1
> and set it to 0 just before cleanup, so that the error gets propagated.
Oh right :)
>
> ACK if you keep reporting the error in OOM case in
> qemuDomainGetStatsInterface()
Done and pushed:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index a7b208f..ed8e140 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -18409,6 +18409,7 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr
driver ATTRIBUTE_UNUSED,
{
size_t i;
struct _virDomainInterfaceStats tmp;
+int ret = -1;
if (!virDomainObjIsActive(dom))
return 0;
@@ -18448,8 +18449,9 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr
driver ATTRIBUTE_UNUSED,
"tx.drop", tmp.tx_drop);
}
+ret = 0;
cleanup:
-return 0;
+return ret;
}
#undef QEMU_ADD_NET_PARAM
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 3/9] conf: new network bridge device attribute macTableManager
On Mon, Dec 08, 2014 at 11:00:03AM -0500, Laine Stump wrote: > The macTableManager attribute of a network's bridge subelement tells > libvirt how the bridge's MAC address table (used to determine the > egress port for packets) is managed. In the default mode, "kernel", > management is left to the kernel, which usually determines entries in > part by turning on promiscuous mode on all ports of the bridge, > flooding packets to all ports when the correct destination is unknown, > and adding/removing entries to the fdb as it sees incoming traffic > from particular MAC addresses. In "libvirt" mode, libvirt turns off > learning and flooding on all the bridge ports connected to guest > domain interfaces, and adds/removes entries according to the MAC > addresses in the domain interface configurations. A side effect of > turning off learning and unicast_flood on the ports of a bridge is > that (with Linux kernel 3.17 and newer), the kernel can automatically > turn off promiscuous mode on one or more of the bridge's ports > (usually only the one interface that is used to connect the bridge to > the physical network). The result is better performance (because > packets aren't being flooded to all ports, and can be dropped earlier > when they are of no interest) and slightly better security (a guest > can still send out packets with a spoofed source MAC address, but will > only receive traffic intended for the guest interface's configured MAC > address). > > The attribute looks like this in the configuration: > > > test > > ... > > This patch only adds the config knob, documentation, and test > cases. The functionality behind this knob is added in later patches. ACK, design looks good now. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/9] virstoragefile: Add quorum in virstoragefile
Add VIR_STORAGE_TYPE_QUORUM in virStorageType.
Add VIR_STORAGE_FILE_QUORUM in virStorageFileFormat.
Add threshold value in _virStorageSource
Signed-off-by: Matthias Gatto
---
src/conf/domain_conf.c| 2 ++
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_driver.c| 4
src/qemu/qemu_migration.c | 1 +
src/util/virstoragefile.c | 25 +
src/util/virstoragefile.h | 3 +++
6 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 00e0470..1add21f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5445,6 +5445,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
if (virDomainDiskSourcePoolDefParse(node, &src->srcpool) < 0)
goto cleanup;
break;
+case VIR_STORAGE_TYPE_QUORUM:
case VIR_STORAGE_TYPE_NONE:
case VIR_STORAGE_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -16412,6 +16413,7 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
skipSeclabels);
break;
+case VIR_STORAGE_TYPE_QUORUM:
case VIR_STORAGE_TYPE_NONE:
case VIR_STORAGE_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1831323..525a49c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3086,6 +3086,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
goto cleanup;
break;
+case VIR_STORAGE_TYPE_QUORUM:
case VIR_STORAGE_TYPE_VOLUME:
case VIR_STORAGE_TYPE_NONE:
case VIR_STORAGE_TYPE_LAST:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8e3a492..1dc7558 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12977,6 +12977,7 @@
qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)
}
break;
+case VIR_STORAGE_TYPE_QUORUM:
case VIR_STORAGE_TYPE_DIR:
case VIR_STORAGE_TYPE_VOLUME:
case VIR_STORAGE_TYPE_NONE:
@@ -13040,6 +13041,7 @@
qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
}
break;
+case VIR_STORAGE_TYPE_QUORUM:
case VIR_STORAGE_TYPE_DIR:
case VIR_STORAGE_TYPE_VOLUME:
case VIR_STORAGE_TYPE_NONE:
@@ -13064,6 +13066,7 @@
qemuDomainSnapshotPrepareDiskExternalOverlayInactive(virDomainSnapshotDiskDefPtr
case VIR_STORAGE_TYPE_FILE:
return 0;
+case VIR_STORAGE_TYPE_QUORUM:
case VIR_STORAGE_TYPE_NETWORK:
case VIR_STORAGE_TYPE_DIR:
case VIR_STORAGE_TYPE_VOLUME:
@@ -13183,6 +13186,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr
conn,
}
break;
+case VIR_STORAGE_TYPE_QUORUM:
case VIR_STORAGE_TYPE_DIR:
case VIR_STORAGE_TYPE_VOLUME:
case VIR_STORAGE_TYPE_NONE:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 0acbb57..5efd753 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1515,6 +1515,7 @@ qemuMigrationPrecreateDisk(virConnectPtr conn,
case VIR_STORAGE_TYPE_BLOCK:
case VIR_STORAGE_TYPE_DIR:
case VIR_STORAGE_TYPE_NETWORK:
+case VIR_STORAGE_TYPE_QUORUM:
case VIR_STORAGE_TYPE_NONE:
case VIR_STORAGE_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 4217a80..ad87e80 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -57,7 +57,8 @@ VIR_ENUM_IMPL(virStorage, VIR_STORAGE_TYPE_LAST,
"block",
"dir",
"network",
- "volume")
+ "volume",
+ "quorum")
VIR_ENUM_IMPL(virStorageFileFormat,
VIR_STORAGE_FILE_LAST,
@@ -66,7 +67,7 @@ VIR_ENUM_IMPL(virStorageFileFormat,
"cloop", "dmg", "iso",
"vpc", "vdi",
/* Not direct file formats, but used for various drivers */
- "fat", "vhd", "ploop",
+ "fat", "vhd", "ploop", "quorum",
/* Formats with backing file below here */
"cow", "qcow", "qcow2", "qed", "vmdk")
@@ -1911,6 +1912,7 @@ virStorageSourceCopy(const virStorageSource *src,
bool backingChain)
{
virStorageSourcePtr ret = NULL;
+size_t i;
if (VIR_ALLOC(ret) < 0)
return NULL;
@@ -1922,6 +1924,8 @@ virStorageSourceCopy(const virStorageSource *src,
ret->capacity = src->capacity;
ret->readonly = src->readonly;
ret->shared = src->shared;
+ret->nBackingStores = src->nBackingStores;
+ret->threshold = src->threshold;
/* storage driver metadata are not copied */
ret->drv = NULL;
@@ -1970,12 +1974,14 @@ virStorageSourceCopy(const virStorageSource *src,
!(ret->auth = virStorageAuthDefCopy(src->auth)))
goto error;
-if (backingChain && virStorageSourceGetBacking
[libvirt] [PATCH 2/9] virstoragefile: Always use virStorageSourceGetBackingStore to get backing store
Uniformize backing store usage by calling virStorageSourceGetBackingStore
instead of setting backing store manually.
Signed-off-by: Matthias Gatto
---
src/conf/domain_conf.c| 7 ---
src/conf/storage_conf.c | 4 ++--
src/qemu/qemu_cgroup.c| 4 ++--
src/qemu/qemu_domain.c| 2 +-
src/qemu/qemu_driver.c| 12 ++--
src/security/security_dac.c | 2 +-
src/security/security_selinux.c | 4 ++--
src/security/virt-aa-helper.c | 2 +-
src/storage/storage_backend.c | 12 ++--
src/storage/storage_backend_fs.c | 8
src/storage/storage_backend_logical.c | 2 +-
src/util/virstoragefile.c | 20 ++--
tests/virstoragetest.c| 14 +++---
13 files changed, 47 insertions(+), 46 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d81c37..b790fc5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16473,7 +16473,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
/* We currently don't output seclabels for backing chain element */
if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0 ||
virDomainDiskBackingStoreFormat(buf,
-backingStore->backingStore,
+
virStorageSourceGetBackingStore(backingStore, 0),
backingStore->backingStoreRaw,
idx + 1) < 0)
return -1;
@@ -16595,7 +16595,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
/* Don't format backingStore to inactive XMLs until the code for
* persistent storage of backing chains is ready. */
if (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
-virDomainDiskBackingStoreFormat(buf, def->src->backingStore,
+virDomainDiskBackingStoreFormat(buf,
+
virStorageSourceGetBackingStore(def->src, 0),
def->src->backingStoreRaw, 1) < 0)
return -1;
@@ -20554,7 +20555,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
}
}
-for (tmp = disk->src; tmp; tmp = tmp->backingStore) {
+for (tmp = disk->src; tmp; tmp = virStorageSourceGetBackingStore(tmp, 0)) {
int actualType = virStorageSourceGetActualType(tmp);
/* execute the callback only for local storage */
if (actualType != VIR_STORAGE_TYPE_NETWORK &&
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 3987470..a8a90b4 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1642,9 +1642,9 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
&def->target, "target") < 0)
goto cleanup;
-if (def->target.backingStore &&
+if (virStorageSourceGetBackingStore(&(def->target), 0) &&
virStorageVolTargetDefFormat(options, &buf,
- def->target.backingStore,
+
virStorageSourceGetBackingStore(&(def->target), 0),
"backingStore") < 0)
goto cleanup;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 0e94cae..f878f4b 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -120,7 +120,7 @@ qemuSetupDiskCgroup(virDomainObjPtr vm,
virStorageSourcePtr next;
bool forceReadonly = false;
-for (next = disk->src; next; next = next->backingStore) {
+for (next = disk->src; next; next = virStorageSourceGetBackingStore(next,
0)) {
if (qemuSetImageCgroupInternal(vm, next, false, forceReadonly) < 0)
return -1;
@@ -138,7 +138,7 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm,
{
virStorageSourcePtr next;
-for (next = disk->src; next; next = next->backingStore) {
+for (next = disk->src; next; next = virStorageSourceGetBackingStore(next,
0)) {
if (qemuSetImageCgroup(vm, next, true) < 0)
return -1;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 220304f..9157e4d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2708,7 +2708,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
if (virStorageSourceIsEmpty(disk->src))
goto cleanup;
-if (disk->src->backingStore) {
+if (virStorageSourceGetBackingStore(disk->src, 0)) {
if (force_probe)
virStorageSourceBackingStoreClear(disk->src);
else
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9152cf5..9ed5035 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13496,13 +13496,13 @@
qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
/* Update vm in place to match changes. */
tmp = disk->src;
-disk->src = tmp->backingStore;
+disk->src = virSto
[libvirt] [PATCH 7/9] domain_conf: Read and Write quorum config
Add the capabiltty to libvirt to parse and format the quorum syntax
as described here:
http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html
Signed-off-by: Matthias Gatto
---
src/conf/domain_conf.c | 165 +++--
1 file changed, 120 insertions(+), 45 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1add21f..9164cf3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5469,20 +5469,58 @@ virDomainDiskSourceParse(xmlNodePtr node,
}
+static bool
+virDomainDiskThresholdParse(virStorageSourcePtr src,
+xmlNodePtr node)
+{
+char *threshold = virXMLPropString(node, "threshold");
+int ret;
+
+if (!threshold) {
+virReportError(VIR_ERR_XML_ERROR,
+ "%s", _("missing threshold in quorum"));
+return false;
+}
+ret = virStrToLong_ul(threshold, NULL, 10, &src->threshold);
+if (ret < 0 || src->threshold < 2) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unexpected threshold %s"),
+ "threshold must be a decimal number superior to 2 "
+ "and inferior to the number of children");
+VIR_FREE(threshold);
+return false;
+}
+VIR_FREE(threshold);
+return true;
+}
+
+
static int
virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ xmlNodePtr node,
+ size_t pos)
{
virStorageSourcePtr backingStore = NULL;
xmlNodePtr save_ctxt = ctxt->node;
-xmlNodePtr source;
+xmlNodePtr source = NULL;
+xmlNodePtr cur = NULL;
char *type = NULL;
char *format = NULL;
int ret = -1;
+size_t i = 0;
-if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) {
-ret = 0;
-goto cleanup;
+if (src->type == VIR_STORAGE_TYPE_QUORUM) {
+if (!node) {
+ret = 0;
+goto cleanup;
+}
+ctxt->node = node;
+} else {
+if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) {
+ret = 0;
+goto cleanup;
+}
}
if (VIR_ALLOC(backingStore) < 0)
@@ -5514,6 +5552,22 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
goto cleanup;
}
+if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) {
+if (!virDomainDiskThresholdParse(backingStore, node))
+goto cleanup;
+
+for (cur = node->children, i = 0; cur != NULL; cur = cur->next) {
+if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) {
+if (virStorageSourcePushBackingStore(backingStore) == false)
+goto cleanup;
+if ((virDomainDiskBackingStoreParse(ctxt, backingStore, cur,
i) < 0))
+goto cleanup;
+++i;
+}
+}
+goto exit;
+}
+
if (!(source = virXPathNode("./source", ctxt))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing disk backing store source"));
@@ -5521,10 +5575,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
}
if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 ||
-virDomainDiskBackingStoreParse(ctxt, backingStore) < 0)
+virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0)
goto cleanup;
-virStorageSourceSetBackingStore(src, backingStore, 0);
+ exit:
+virStorageSourceSetBackingStore(src, backingStore, pos);
ret = 0;
cleanup:
@@ -5536,7 +5591,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
return ret;
}
-
#define VENDOR_LEN 8
#define PRODUCT_LEN 16
@@ -6030,6 +6084,12 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
} else if (xmlStrEqual(cur->name, BAD_CAST "boot")) {
/* boot is parsed as part of virDomainDeviceInfoParseXML */
+} else if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) {
+if (virStorageSourcePushBackingStore(def->src) == false)
+goto error;
+if (virDomainDiskBackingStoreParse(ctxt, def->src, cur,
+ def->src->nBackingStores -
1) < 0)
+goto error;
}
}
cur = cur->next;
@@ -6053,12 +6113,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
def->device = VIR_DOMAIN_DISK_DEVICE_DISK;
}
+if (def->src->type == VIR_STORAGE_TYPE_QUORUM &&
+!virDomainDiskThresholdParse(def->src, node))
+goto error;
+
+snapshot = virXMLPropString(node, "snapshot");
+
/* Only CDROM and Floppy devices are allowed missing source path
* to indicate no media present. LUN is for raw
[libvirt] [PATCH 9/9] virstoragefile: Add node-name
Add nodename inside virstoragefile
During xml backingStore parsing, look for a nodename attribute in the disk
declaration if this one is a quorum, if a nodename is found, add it to
the virStorageSource otherwise create a new one with a random name.
Take inspiration from this patch to create the nodename:
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03209.html
Durring xml backingStore formating, look for a nodename attribute inside the
virStorageSource struct, and add it to the disk element.
Use the nodename to create the quorum in qemuBuildQuorumStr.
Signed-off-by: Matthias Gatto
---
src/conf/domain_conf.c| 25 +
src/qemu/qemu_command.c | 3 +++
src/util/virstoragefile.c | 4
src/util/virstoragefile.h | 1 +
4 files changed, 33 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9164cf3..a572516 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -53,6 +53,7 @@
#include "device_conf.h"
#include "virtpm.h"
#include "virstring.h"
+#include "virrandom.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -5495,6 +5496,8 @@ virDomainDiskThresholdParse(virStorageSourcePtr src,
}
+#define GEN_NODE_NAME_PREFIX"libvirt"
+#define GEN_NODE_NAME_MAX_LEN (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8)
static int
virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virStorageSourcePtr src,
@@ -5506,6 +5509,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
xmlNodePtr source = NULL;
xmlNodePtr cur = NULL;
char *type = NULL;
+char *nodeName = NULL;
char *format = NULL;
int ret = -1;
size_t i = 0;
@@ -5539,6 +5543,23 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
goto cleanup;
}
+if (src->type == VIR_STORAGE_TYPE_QUORUM) {
+if ((nodeName = virXMLPropString(ctxt->node, "nodename"))) {
+backingStore->nodeName = nodeName;
+} else {
+int len;
+if (VIR_ALLOC_N(nodeName, GEN_NODE_NAME_MAX_LEN) < 0)
+goto cleanup;
+snprintf(nodeName, GEN_NODE_NAME_MAX_LEN,
+ "%s%08x", GEN_NODE_NAME_PREFIX, (int)pos);
+len = strlen(nodeName);
+while (len < GEN_NODE_NAME_MAX_LEN - 1)
+nodeName[len++] = virRandomInt('Z' - 'A') + 'A';
+nodeName[GEN_NODE_NAME_MAX_LEN - 1] = '\0';
+backingStore->nodeName = nodeName;
+}
+}
+
if (!(format = virXPathString("string(./format/@type)", ctxt))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing disk backing store format"));
@@ -5590,6 +5611,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
ctxt->node = save_ctxt;
return ret;
}
+#undef GEN_NODE_NAME_PREFIX
+#undef GEN_NODE_NAME_MAX_LEN
#define VENDOR_LEN 8
#define PRODUCT_LEN 16
@@ -16539,6 +16562,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
type, idx);
if (backingStore->threshold)
virBufferAsprintf(buf, " threshold='%lu'",
backingStore->threshold);
+if (backingStore->nodeName)
+virBufferAsprintf(buf, " nodename='%s'", backingStore->nodeName);
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 03ff1b7..77c38c4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3255,6 +3255,9 @@ qemuBuildQuorumStr(virConnectPtr conn,
toAppend, i,
virStorageFileFormatTypeToString(backingStore->format));
+virBufferAsprintf(opt, ",%schildren.%lu.node-name=%s",
+ toAppend, i, backingStore->nodeName);
+
if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) ==
false)
goto error;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index ad87e80..cd0098c 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1974,6 +1974,9 @@ virStorageSourceCopy(const virStorageSource *src,
!(ret->auth = virStorageAuthDefCopy(src->auth)))
goto error;
+if (src->nodeName && VIR_STRDUP(ret->nodeName, src->nodeName) < 0)
+goto error;
+
for (i = 0; i < src->nBackingStores; ++i) {
if (backingChain && virStorageSourceGetBackingStore(src, i)) {
if (!virStorageSourceSetBackingStore(ret,
@@ -2119,6 +2122,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def)
VIR_FREE(def->relPath);
VIR_FREE(def->backingStoreRaw);
+VIR_FREE(def->nodeName);
/* recursively free backing chain */
for (i = 0; i < def->nBackingStores; ++i)
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 40d221c..b53dd70 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -274,6 +274,7 @@ struct _virStorageSource {
[libvirt] [PATCH 0/9] qemu: Add quorum support to libvirt
The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html This feature ask for 6 task: 1) Allow a _virStorageSource to contain more than one backing store. Therefore we have to treat the field virStorageSourcePtr backingStores as an array instead of a pointer. But doing that, most of the backingStore field would be an array contening only one element. So I've decide to allocate the array only if there is more than 1 backing store in a _virStorageSource. Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation. 2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums. 3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse in a loop. virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can call themself recursively in a loop because a quorum can contain another quorum 4) Add nodename We need to add nodename support in _virStorageSource because qemu use them for their child. 5) Build qemu string As for the xml, we have to call the function which create quorum recursively. But this task have the problem explained here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html The _virStorageSource missing some informations that can be passed to a child, and therefore this version of quorum is incomplet. 6) Allow to hotplug/change a disk in a quorum This part is not present in these patches because for this task we have to use blockdev-add, and currently libvirt use device_add for hotpluging that doesn't allow to hotplug quorum childs. There is 3 way to handle this problem: 1) create a virDomainBlockDevAdd function in libvirt witch call blockdev-add. 2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice 3) write a hack which uses blockdev-add when only attaching quorum (but i'm pretty sure this solution is not the good one) Matthias Gatto (9): virstoragefile: Add virStorageSourceGetBackingStore virstoragefile: Always use virStorageSourceGetBackingStore to get backing store virstoragefile: Add virStorageSourceSetBackingStore virstoragefile: Always use virStorageSourceSetBackingStore to set backing store virstoragefile: Treat backingStore as a pointer or an array virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name src/conf/domain_conf.c| 193 ++ src/conf/storage_conf.c | 7 +- src/libvirt_private.syms | 3 + src/qemu/qemu_cgroup.c| 4 +- src/qemu/qemu_command.c | 114 src/qemu/qemu_domain.c| 3 +- src/qemu/qemu_driver.c| 20 ++-- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 12 +-- src/storage/storage_backend_fs.c | 12 +-- src/storage/storage_backend_logical.c | 4 +- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 136 +--- src/util/virstoragefile.h | 12 +++ tests/virstoragetest.c| 18 ++-- 17 files changed, 445 insertions(+), 103 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/9] qemu: Add quorum support in qemuBuildDriveDevStr
Allow to libvirt to build the quorum string used by quemu.
Add 2 static functions: qemuBuildQuorumStr and
qemuBuildAndAppendDriveStrToVirBuffer.
qemuBuildQuorumStr is made because a quorum can have another quorum
as a child, so we may need to call qemuBuildQuorumStr recursively.
qemuBuildQuorumFileSourceStr was basically made to share
the code use to build the source between qemuBuildQuorumStr and
qemuBuildDriveStr, but there is some difference betwin the syntax
use by libvirt to declare a disk and the one qemu need to build a quorum:
a quorum need a syntaxe like:
"domaine_name.children.X.file.filename=filename"
where libvirt don't use "file.filename=" but directly "file=".
Therfore I use this function only for quorum.
But as explained in the cover letter and here:
http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html
We miss some informations in _virStorageSource to have a complet
quorum support in libvirt.
Ideally I'd like to refactore virDomainDiskDefFormat to allow
qemuBuildQuorumStr to call this function in a loop.
Signed-off-by: Matthias Gatto
---
src/qemu/qemu_command.c | 110
1 file changed, 110 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 525a49c..03ff1b7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3167,6 +3167,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
return -1;
}
+static bool
+qemuBuildQuorumFileSourceStr(virConnectPtr conn,
+ virStorageSourcePtr src,
+ virBuffer *opt,
+ const char *toAppend)
+{
+char *source = NULL;
+int actualType = virStorageSourceGetActualType(src);
+
+if (qemuGetDriveSourceString(src, conn, &source) < 0)
+goto error;
+
+if (source) {
+
+virBufferAsprintf(opt, ",%sfilename=", toAppend);
+
+switch (actualType) {
+case VIR_STORAGE_TYPE_DIR:
+/* QEMU only supports magic FAT format for now */
+if (src->format > 0 &&
+src->format != VIR_STORAGE_FILE_FAT) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unsupported disk driver type for '%s'"),
+ virStorageFileFormatTypeToString(src->format));
+goto error;
+}
+
+if (!src->readonly) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot create virtual FAT disks in
read-write mode"));
+goto error;
+}
+
+virBufferAddLit(opt, "fat:");
+
+break;
+
+default:
+break;
+}
+virBufferAsprintf(opt, "%s", source);
+}
+
+return true;
+ error:
+return false;
+}
+
+
+static bool
+qemuBuildQuorumStr(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ virStorageSourcePtr src,
+ virBuffer *opt,
+ const char *toAppend)
+{
+char *tmp = NULL;
+int ret;
+virStorageSourcePtr backingStore;
+size_t i;
+
+if (!src->threshold) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("threshold missing in the quorum configuration"));
+return false;
+}
+if (src->nBackingStores < 2) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("a quorum must have at last 2 children"));
+return false;
+}
+if (src->threshold > src->nBackingStores) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("threshold must not exceed the number of childrens"));
+return false;
+}
+virBufferAsprintf(opt, ",%svote-threshold=%lu",
+ toAppend, src->threshold);
+for (i = 0; i < src->nBackingStores; ++i) {
+backingStore = virStorageSourceGetBackingStore(src, i);
+ret = virAsprintf(&tmp, "%schildren.%lu.file.", toAppend, i);
+if (ret < 0)
+return false;
+
+virBufferAsprintf(opt, ",%schildren.%lu.driver=%s",
+ toAppend, i,
+
virStorageFileFormatTypeToString(backingStore->format));
+
+if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) ==
false)
+goto error;
+
+/* This operation avoid me to made another copy */
+tmp[ret - sizeof("file")] = '\0';
+if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) {
+if (!qemuBuildQuorumStr(conn, disk, backingStore, opt, tmp))
+goto error;
+}
+VIR_FREE(tmp);
+}
+return true;
+ error:
+VIR_FREE(tmp);
+return false;
+}
+
/* Qemu 1.2 and later have a binary flag -enable-fips that must be
* used for VNC auth to obey FIPS settings; but the flag only
@@ -3639,6 +3744,11 @@ qemuBuildDriveStr(virConnect
[libvirt] [PATCH 1/9] virstoragefile: Add virStorageSourceGetBackingStore
Create virStorageSourceGetBackingStore function in
preparation for quorum:
Actually, if we want to get a backing store inside a virStorageSource
we have to do it manually(src->backingStore = backingStore).
The problem is that with a quorum, a virStorageSource
can contain multiple backing stores, and src->backingStore can
be treated as a virStorageSourcePtr or a virStorageSourcePtrPtr.
Due to these reason, we need to simplify the manipulation of
virStorageSource, and create a function to get the asked
backingStore in a virStorageSource
For now, this function only return the backingStore field
Signed-off-by: Matthias Gatto
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 8
src/util/virstoragefile.h | 3 +++
3 files changed, 12 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1853a9c..12dd399 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1992,6 +1992,7 @@ virStorageSourceClear;
virStorageSourceCopy;
virStorageSourceFree;
virStorageSourceGetActualType;
+virStorageSourceGetBackingStore;
virStorageSourceGetSecurityLabelDef;
virStorageSourceInitChainElement;
virStorageSourceIsEmpty;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index c424251..ebcf7c3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1814,6 +1814,14 @@ virStorageSourcePoolDefCopy(const
virStorageSourcePoolDef *src)
}
+virStorageSourcePtr
+virStorageSourceGetBackingStore(const virStorageSource *src,
+size_t pos ATTRIBUTE_UNUSED)
+{
+return src->backingStore;
+}
+
+
/**
* virStorageSourcePtr:
*
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index e05b843..fe07dde 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -288,6 +288,9 @@ struct _virStorageSource {
# define DEV_BSIZE 512
# endif
+virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource
*src,
+size_t pos);
+
int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);
int virStorageFileProbeFormatFromBuf(const char *path,
char *buf,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] virstoragefile: Treat backingStore as a pointer or an array
As explain in the former patchs, backingStore can be treat an array or
a pointer.
If we have only one backingStore we have to use it as a normal ptr
but if there is more backing store, we use it as a pointer's array.
Because it would be complicated to expend backingStore manually, and do
the convertion from a pointer to an array, I've created the
virStorageSourcePushBackingStore function to help.
This function allocate an array of pointer only if there is more than 1 bs.
Because we can not remove a child from a quorum, i didn't create the function
virStorageSourcePopBackingStore.
I've changed virStorageSourceBackingStoreClear, virStorageSourceSetBackingStore
and virStorageSourceGetBackingStore to handle the case where backingStore
is an array.
Signed-off-by: Matthias Gatto
---
src/conf/storage_conf.c | 3 +-
src/libvirt_private.syms | 1 +
src/storage/storage_backend_fs.c | 2 +-
src/storage/storage_backend_logical.c | 2 +-
src/util/virstoragefile.c | 87 ---
src/util/virstoragefile.h | 2 +
6 files changed, 87 insertions(+), 10 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index a8a90b4..fc51d47 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1340,8 +1340,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
}
if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) {
-if (VIR_ALLOC(ret->target.backingStore) < 0)
+if (VIR_ALLOC_N(ret->target.backingStore, 1) < 0)
goto error;
+ret->target.nBackingStores = 1;
ret->target.backingStore->path = backingStore;
backingStore = NULL;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index dd83518..71e8625 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2002,6 +2002,7 @@ virStorageSourceParseRBDColonString;
virStorageSourcePoolDefFree;
virStorageSourcePoolModeTypeFromString;
virStorageSourcePoolModeTypeToString;
+virStorageSourcePushBackingStore;
virStorageSourceSetBackingStore;
virStorageTypeFromString;
virStorageTypeToString;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 07fd6ee..979abd3 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -107,7 +107,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
if
(!virStorageSourceIsLocalStorage(virStorageSourceGetBackingStore(target, 0))) {
virStorageSourceFree(virStorageSourceGetBackingStore(target, 0));
-if (VIR_ALLOC(target->backingStore) < 0)
+if (virStorageSourcePushBackingStore(target) == false)
goto cleanup;
target->backingStore->type = VIR_STORAGE_TYPE_NETWORK;
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 300c990..edec124 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -148,7 +148,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
* lv is created with "--virtualsize").
*/
if (groups[1] && !STREQ(groups[1], "") && (groups[1][0] != '[')) {
-if (VIR_ALLOC(vol->target.backingStore) < 0)
+if (virStorageSourcePushBackingStore(&vol->target) == false)
goto cleanup;
if (virAsprintf(&vol->target.backingStore->path, "%s/%s",
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 20f45f3..4217a80 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1814,21 +1814,86 @@ virStorageSourcePoolDefCopy(const
virStorageSourcePoolDef *src)
}
+/**
+ * virStorageSourceGetBackingStore:
+ * @src: virStorageSourcePtr containing the backing stores
+ * @pos: position of the backing store to get
+ *
+ * return the backingStore at the position of @pos
+ */
virStorageSourcePtr
-virStorageSourceGetBackingStore(const virStorageSource *src,
-size_t pos ATTRIBUTE_UNUSED)
+virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos)
{
-return src->backingStore;
+if (!src->backingStore || (pos > 1 && pos >= src->nBackingStores))
+return NULL;
+if (src->nBackingStores < 2)
+return src->backingStore;
+return ((virStorageSourcePtr *)src->backingStore)[pos];
}
+/**
+ * virStorageSourcePushBackingStore:
+ * @src: virStorageSourcePtr to allocate the new backing store
+ *
+ * Allocate size for a new backing store in src->backingStore
+ * and update src->nBackingStores
+ * If we have less than 2 backing stores, we treat src->backingStore
+ * as a pointer otherwise we treat it as an array of virStorageSourcePtr
+ */
+bool
+virStorageSourcePushBackingStore(virStorageSourcePtr src)
+{
+virStorageSourcePtr tmp;
+virStorageSourcePtr *tmp2;
+
+if (src->nBackingStores == 1) {
+/* If we need more than one
[libvirt] [PATCH 3/9] virstoragefile: Add virStorageSourceSetBackingStore
As explained for virStorageSourceGetBackingStore, quorum allows
multiple backing store, this make the operation to set bs complex
because we have to check if the backingStore is used as an array
or a pointer, and set it differently in both case.
In order to help the manipulation of backing store, I've added a
function virStorageSourceSetBackingStore.
For now virStorageSourceSetBackingStore don't handle the case where
we have more than one backing store in virStorageSource.
Signed-off-by: Matthias Gatto
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 10 ++
src/util/virstoragefile.h | 3 +++
3 files changed, 14 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 12dd399..dd83518 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2002,6 +2002,7 @@ virStorageSourceParseRBDColonString;
virStorageSourcePoolDefFree;
virStorageSourcePoolModeTypeFromString;
virStorageSourcePoolModeTypeToString;
+virStorageSourceSetBackingStore;
virStorageTypeFromString;
virStorageTypeToString;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index fd0e68c..88b0ce6 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1822,6 +1822,16 @@ virStorageSourceGetBackingStore(const virStorageSource
*src,
}
+virStorageSourcePtr
+virStorageSourceSetBackingStore(virStorageSourcePtr src,
+virStorageSourcePtr backingStore,
+size_t pos ATTRIBUTE_UNUSED)
+{
+src->backingStore = backingStore;
+return src->backingStore;
+}
+
+
/**
* virStorageSourcePtr:
*
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index fe07dde..4cba66b 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -288,6 +288,9 @@ struct _virStorageSource {
# define DEV_BSIZE 512
# endif
+virStorageSourcePtr virStorageSourceSetBackingStore(virStorageSourcePtr src,
+virStorageSourcePtr
backingStore,
+size_t pos);
virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource
*src,
size_t pos);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/9] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store
Replace the parts of the code where a backing store is set manually
with virStorageSourceSetBackingStore
Signed-off-by: Matthias Gatto
---
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_domain.c | 1 -
src/qemu/qemu_driver.c | 4 ++--
src/storage/storage_backend_fs.c | 2 +-
src/storage/storage_driver.c | 2 +-
src/util/virstoragefile.c| 8 +---
tests/virstoragetest.c | 4 ++--
7 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b790fc5..00e0470 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5523,7 +5523,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virDomainDiskBackingStoreParse(ctxt, backingStore) < 0)
goto cleanup;
-src->backingStore = backingStore;
+virStorageSourceSetBackingStore(src, backingStore, 0);
ret = 0;
cleanup:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9157e4d..458ceb0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2534,7 +2534,6 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
int ret = -1;
size_t i;
-VIR_DEBUG("Checking for disk presence");
for (i = vm->def->ndisks; i > 0; i--) {
size_t idx = i - 1;
virDomainDiskDefPtr disk = vm->def->disks[idx];
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9ed5035..8e3a492 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13497,13 +13497,13 @@
qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
/* Update vm in place to match changes. */
tmp = disk->src;
disk->src = virStorageSourceGetBackingStore(tmp, 0);
-tmp->backingStore = NULL;
+virStorageSourceSetBackingStore(tmp, NULL, 0);
virStorageSourceFree(tmp);
if (persistDisk) {
tmp = persistDisk->src;
persistDisk->src = virStorageSourceGetBackingStore(tmp, 0);
-tmp->backingStore = NULL;
+virStorageSourceSetBackingStore(tmp, NULL, 0);
virStorageSourceFree(tmp);
}
}
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index d7bd1fb..07fd6ee 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -96,7 +96,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
goto cleanup;
if (meta->backingStoreRaw) {
-if (!(target->backingStore = virStorageSourceNewFromBacking(meta)))
+if (!virStorageSourceSetBackingStore(target,
virStorageSourceNewFromBacking(meta), 0))
goto cleanup;
target->backingStore->format = backingStoreFormat;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 66dc994..eeb2a4c 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2899,7 +2899,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
goto cleanup;
}
-src->backingStore = backingStore;
+virStorageSourceSetBackingStore(src, backingStore, 0);
backingStore = NULL;
ret = 0;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 88b0ce6..20f45f3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1906,8 +1906,10 @@ virStorageSourceCopy(const virStorageSource *src,
goto error;
if (backingChain && virStorageSourceGetBackingStore(src, 0)) {
-if (!(ret->backingStore =
virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0),
- true)))
+if (!virStorageSourceSetBackingStore(ret,
+
virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0),
+ true),
+ 0))
goto error;
}
@@ -2044,7 +2046,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def)
/* recursively free backing chain */
virStorageSourceFree(virStorageSourceGetBackingStore(def, 0));
-def->backingStore = NULL;
+virStorageSourceSetBackingStore(def, NULL, 0);
}
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 12670da..b7820d4 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -583,9 +583,9 @@ testPathRelativePrepare(void)
for (i = 0; i < ARRAY_CARDINALITY(backingchain); i++) {
if (i < ARRAY_CARDINALITY(backingchain) - 1)
-backingchain[i].backingStore = &backingchain[i + 1];
+virStorageSourceSetBackingStore(&backingchain[i], &backingchain[i
+ 1], 0);
else
-backingchain[i].backingStore = NULL;
+virStorageSourceSetBackingStore(&backingchain[i], NULL, 0);
backingchain[i].relPath = NULL;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 6/9] network: setup bridge devices for macTableManager='libvirt'
When the bridge device for a network has macTableManager='libvirt' the
intent is that all kernel management of the bridge's MAC table
(Forwarding Database, or fdb, in the case of a Linux Host Bridge) be
disabled, with libvirt handling updates to the table instead. The
setup required for the bridge itself is:
1) set the "vlan_filtering" property of the bridge device to 1.
2) If the bridge has a "Dummy" tap device used to set a fixed MAC
address on the bridge (which is always the case for a bridge created
by libvirt, and never the case for a bridge created by the host system
network config), turn off learning and unicast_flood on this tap (this
is needed even though this tap is never IFF_UP, because the kernel
ignores the IFF_UP flag of devices when using their settings to
automatically decide whether or not to turn off promiscuous mode for
any attached device).
(1) is done both for libvirt-created/managed bridges, and for bridges
that are created by the host system config, while (2) is done only for
bridges created by libvirt (i.e. for forward modes of nat, routed, and
isolated bridges)
There is no attempt to turn vlan_filtering off when destroying the
network because in the case of a libvirt-created bridge, the bridge is
about to be destroyed anyway, and in the case of a system bridge, if
the other devices attached to the bridge could operate properly before
destroying libvirt's network object, they will continue to operate
properly (this is similar to the way that libvirt will enable
ip_forwarding whenever a routed/natted network is started, but will
never attempt to disable it if they are stopped).
---
Change from V2: attribute name.
src/network/bridge_driver.c | 54 +
1 file changed, 54 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 93d36ab..29222d0 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1906,6 +1906,29 @@ networkAddAddrToBridge(virNetworkObjPtr network,
return 0;
}
+
+static int
+networkStartHandleMACTableManagerMode(virNetworkObjPtr network,
+ const char *macTapIfName)
+{
+const char *brname = network->def->bridge;
+
+if (brname &&
+network->def->macTableManager
+== VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
+if (virNetDevBridgeSetVlanFiltering(brname, true) < 0)
+return -1;
+if (macTapIfName) {
+if (virNetDevBridgePortSetLearning(brname, macTapIfName, false) <
0)
+return -1;
+if (virNetDevBridgePortSetUnicastFlood(brname, macTapIfName,
false) < 0)
+return -1;
+}
+}
+return 0;
+}
+
+
/* add an IP (static) route to a bridge */
static int
networkAddRouteToBridge(virNetworkObjPtr network,
@@ -2032,6 +2055,9 @@ networkStartNetworkVirtual(virNetworkObjPtr network)
goto err2;
}
+if (networkStartHandleMACTableManagerMode(network, macTapIfName) < 0)
+goto err2;
+
/* Bring up the bridge interface */
if (virNetDevSetOnline(network->def->bridge, 1) < 0)
goto err2;
@@ -2176,6 +2202,27 @@ static int
networkShutdownNetworkVirtual(virNetworkObjPtr network)
}
+static int
+networkStartNetworkBridge(virNetworkObjPtr network)
+{
+/* put anything here that needs to be done each time a network of
+ * type BRIDGE, is started. On failure, undo anything you've done,
+ * and return -1. On success return 0.
+ */
+return networkStartHandleMACTableManagerMode(network, NULL);
+}
+
+static int
+networkShutdownNetworkBridge(virNetworkObjPtr network ATTRIBUTE_UNUSED)
+{
+/* put anything here that needs to be done each time a network of
+ * type BRIDGE is shutdown. On failure, undo anything you've done,
+ * and return -1. On success return 0.
+ */
+return 0;
+}
+
+
/* networkCreateInterfacePool:
* @netdef: the original NetDef from the network
*
@@ -2339,6 +2386,10 @@ networkStartNetwork(virNetworkObjPtr network)
break;
case VIR_NETWORK_FORWARD_BRIDGE:
+ if (networkStartNetworkBridge(network) < 0)
+ goto cleanup;
+ break;
+
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
@@ -2405,6 +2456,9 @@ static int networkShutdownNetwork(virNetworkObjPtr
network)
break;
case VIR_NETWORK_FORWARD_BRIDGE:
+ret = networkShutdownNetworkBridge(network);
+break;
+
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 1/9] util: new functions for setting bridge and bridge port attributes
These functions all set/get items in the sysfs for a bridge device.
---
No change since V1.
src/libvirt_private.syms | 6 ++
src/util/virnetdevbridge.c | 235 -
src/util/virnetdevbridge.h | 28 +-
3 files changed, 266 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1853a9c..7cb57a9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1663,9 +1663,15 @@ virNetDevBridgeCreate;
virNetDevBridgeDelete;
virNetDevBridgeGetSTP;
virNetDevBridgeGetSTPDelay;
+virNetDevBridgeGetVlanFiltering;
+virNetDevBridgePortGetLearning;
+virNetDevBridgePortGetUnicastFlood;
+virNetDevBridgePortSetLearning;
+virNetDevBridgePortSetUnicastFlood;
virNetDevBridgeRemovePort;
virNetDevBridgeSetSTP;
virNetDevBridgeSetSTPDelay;
+virNetDevBridgeSetVlanFiltering;
# util/virnetdevmacvlan.h
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index 15434de..c6a3e8e 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007-2013 Red Hat, Inc.
+ * Copyright (C) 2007-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -219,6 +219,170 @@ static int virNetDevBridgeGet(const char *brname,
}
#endif /* __linux__ */
+#if defined(__linux__)
+static int
+virNetDevBridgePortSet(const char *brname,
+ const char *ifname,
+ const char *paramname,
+ unsigned long value)
+{
+char *path = NULL;
+char valuestr[INT_BUFSIZE_BOUND(value)];
+int ret = -1;
+
+snprintf(valuestr, sizeof(valuestr), "%lu", value);
+
+if (virAsprintf(&path, "%s/%s/brif/%s/%s",
+SYSFS_NET_DIR, brname, ifname, paramname) < 0)
+return -1;
+
+if (!virFileExists(path))
+errno = EINVAL;
+else
+ret = virFileWriteStr(path, valuestr, 0);
+
+if (ret < 0) {
+virReportSystemError(errno,
+ _("Unable to set bridge %s port %s %s to %s"),
+ brname, ifname, paramname, valuestr);
+}
+
+VIR_FREE(path);
+return ret;
+}
+
+
+static int
+virNetDevBridgePortGet(const char *brname,
+ const char *ifname,
+ const char *paramname,
+ unsigned long *value)
+{
+char *path = NULL;
+char *valuestr = NULL;
+int ret = -1;
+
+if (virAsprintf(&path, "%s/%s/brif/%s/%s",
+SYSFS_NET_DIR, brname, ifname, paramname) < 0)
+return -1;
+
+if (virFileReadAll(path, INT_BUFSIZE_BOUND(unsigned long), &valuestr) < 0)
+goto cleanup;
+
+if (virStrToLong_ul(valuestr, NULL, 10, value) < 0) {
+virReportSystemError(EINVAL,
+ _("Unable to get bridge %s port %s %s"),
+ brname, ifname, paramname);
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(path);
+VIR_FREE(valuestr);
+return ret;
+}
+
+
+int
+virNetDevBridgePortGetLearning(const char *brname,
+ const char *ifname,
+ bool *enable)
+{
+int ret = -1;
+unsigned long value;
+
+if (virNetDevBridgePortGet(brname, ifname, "learning", &value) < 0)
+ goto cleanup;
+
+*enable = !!value;
+ret = 0;
+ cleanup:
+return ret;
+}
+
+
+int
+virNetDevBridgePortSetLearning(const char *brname,
+ const char *ifname,
+ bool enable)
+{
+return virNetDevBridgePortSet(brname, ifname, "learning", enable ? 1 : 0);
+}
+
+
+int
+virNetDevBridgePortGetUnicastFlood(const char *brname,
+ const char *ifname,
+ bool *enable)
+{
+int ret = -1;
+unsigned long value;
+
+if (virNetDevBridgePortGet(brname, ifname, "unicast_flood", &value) < 0)
+ goto cleanup;
+
+*enable = !!value;
+ret = 0;
+ cleanup:
+return ret;
+}
+
+
+int
+virNetDevBridgePortSetUnicastFlood(const char *brname,
+ const char *ifname,
+ bool enable)
+{
+return virNetDevBridgePortSet(brname, ifname, "unicast_flood", enable ? 1
: 0);
+}
+
+
+#else
+int
+virNetDevBridgePortGetLearning(const char *brname ATTRIBUTE_UNUSED,
+ const char *ifname ATTRIBUTE_UNUSED,
+ bool *enable ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, "%s",
+ _("Unable to get bridge port learning on this
platform"));
+return -1;
+}
+
+
+int
+virNetDevBridgePortSetLearning(const char *brname ATTRIBUTE_UNUSED,
+ const char *ifname ATTRIBUTE_UNUSED,
+ bool enable)
+{
+virReportSy
[libvirt] [PATCHv3 7/9] qemu: setup tap devices for macTableManager='libvirt'
When libvirt is managing the MAC table of a Linux host bridge, it must
turn off learning and unicast_flood for each tap device attached to
that bridge, then add a Forwarding Database (fdb) entry for the tap
device using the MAC address from the domain interface config.
Once we have disabled learning and flooding, any packet that has a
destination MAC address not present in the fdb will be dropped by the
bridge. This, along with the opportunistic disabling of promiscuous
mode[*], can result in enhanced network performance. and a potential
slight security improvement.
[*] If there is only one device on the bridge with learning/unicast_flood
enabled, then that device will automatically have promiscuous mode
disabled. If there are *no* devices with learning/unicast_flood
enabled (e.g. for a libvirt "route", "nat", or isolated network that
has no physical device attached), then all non-tap devices will have
promiscuous mode disabled (tap devices always have promiscuous mode
enabled, which may be a bug in the kernel, but in practice has 0
effect).
None of this has any effect for kernels prior to 3.15 (upstream kernel
commit 2796d0c648c940b4796f84384fbcfb0a2399db84 "bridge: Automatically
manage port promiscuous mode"). Even after that, until kernel 3.17
(upstream commit 5be5a2df40f005ea7fb7e280e87bbbcfcf1c2fc0 "bridge: Add
filtering support for default_pvid") traffic will not be properly
forwarded without manually adding vlan table entries. Unfortunately,
although the presence of the first patch is signalled by existence of
the "learning" and "unicast_flood" options in sysfs, there is no
reliable way to query whether or not the system's kernel has the
second of those patches installed, the only thing that can be done is
to try the setting and see if traffic continues to pass.
---
Change from V2: attribute name and commit log explanation.
src/qemu/qemu_command.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1831323..11ed58b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -35,6 +35,7 @@
#include "virerror.h"
#include "virfile.h"
#include "virnetdev.h"
+#include "virnetdevbridge.h"
#include "virstring.h"
#include "virtime.h"
#include "viruuid.h"
@@ -344,6 +345,24 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
virDomainAuditNetDevice(def, net, tunpath, false);
goto cleanup;
}
+if (virDomainNetGetActualBridgeMACTableManager(net)
+== VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
+/* libvirt is managing the FDB of the bridge this device
+ * is attaching to, so we need to turn off learning and
+ * unicast_flood on the device to prevent the kernel from
+ * adding any FDB entries for it, then add an fdb entry
+ * outselves, using the MAC address from the interface
+ * config.
+ */
+if (virNetDevBridgePortSetLearning(brname, net->ifname, false) < 0)
+goto cleanup;
+if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, false)
< 0)
+goto cleanup;
+if (virNetDevBridgeFDBAdd(&net->mac, net->ifname,
+ VIR_NETDEVBRIDGE_FDB_FLAG_MASTER |
+ VIR_NETDEVBRIDGE_FDB_FLAG_TEMP) < 0)
+goto cleanup;
+}
} else {
if (qemuCreateInBridgePortWithHelper(cfg, brname,
&net->ifname,
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 2/9] util: functions to manage bridge fdb (forwarding database)
These two functions use netlink RTM_NEWNEIGH and RTM_DELNEIGH messages
to add and delete entries from a bridge's fdb. The bridge itself is
not referenced in the arguments to the functions, only the name of the
device that is attached to the bridge (since a device can only be
attached to one bridge at a time, and must be attached for this
function to make sense, the kernel easily infers which bridge's fdb is
being modified by looking at the device name/index).
---
No change from V2.
src/libvirt_private.syms | 2 +
src/util/virnetdevbridge.c | 147 +
src/util/virnetdevbridge.h | 16 +
3 files changed, 165 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7cb57a9..3e445fc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1661,6 +1661,8 @@ virNetDevBandwidthUpdateRate;
virNetDevBridgeAddPort;
virNetDevBridgeCreate;
virNetDevBridgeDelete;
+virNetDevBridgeFDBAdd;
+virNetDevBridgeFDBDel;
virNetDevBridgeGetSTP;
virNetDevBridgeGetSTPDelay;
virNetDevBridgeGetVlanFiltering;
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index c6a3e8e..78c4a25 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -37,6 +37,9 @@
#include
#ifdef __linux__
+# if defined(HAVE_LIBNL)
+# include "virnetlink.h"
+# endif
# include
# include /* HZ */
# if NETINET_LINUX_WORKAROUND
@@ -913,3 +916,147 @@ virNetDevBridgeSetVlanFiltering(const char *brname
ATTRIBUTE_UNUSED,
return -1;
}
#endif
+
+
+#if defined(__linux__) && defined(HAVE_LIBNL)
+/* virNetDevBridgeFDBAddDel:
+ * @mac: the MAC address being added to the table
+ * @ifname: name of the port (interface) of the bridge that wants this MAC
+ * @flags: any of virNetDevBridgeFDBFlags ORed together.
+ * @isAdd: true if adding the entry, fals if deleting
+ *
+ * Use netlink RTM_NEWNEIGH and RTM_DELNEIGH messages to add and
+ * delete entries from a bridge's fdb. The bridge itself is not
+ * referenced in the arguments to the function, only the name of the
+ * device that is attached to the bridge (since a device can only be
+ * attached to one bridge at a time, and must be attached for this
+ * function to make sense, the kernel easily infers which bridge's fdb
+ * is being modified by looking at the device name/index).
+ *
+ * Attempting to add an existing entry, or delete a non-existing entry
+ * *is* an error.
+ *
+ * returns 0 on success, -1 on failure.
+ */
+static int
+virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname,
+ unsigned int flags, bool isAdd)
+{
+int ret = -1;
+struct nlmsghdr *resp = NULL;
+struct nlmsgerr *err;
+unsigned int recvbuflen;
+struct nl_msg *nl_msg;
+struct ndmsg ndm = { .ndm_family = PF_BRIDGE, .ndm_state = NUD_NOARP };
+
+if (virNetDevGetIndex(ifname, &ndm.ndm_ifindex) < 0)
+return -1;
+
+if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_ROUTER)
+ndm.ndm_flags |= NTF_ROUTER;
+if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_SELF)
+ndm.ndm_flags |= NTF_SELF;
+if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_MASTER)
+ndm.ndm_flags |= NTF_MASTER;
+/* default self (same as iproute2's bridge command */
+if (!(ndm.ndm_flags & (NTF_MASTER | NTF_SELF)))
+ndm.ndm_flags |= NTF_SELF;
+
+if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_PERMANENT)
+ndm.ndm_state |= NUD_PERMANENT;
+if (flags & VIR_NETDEVBRIDGE_FDB_FLAG_TEMP)
+ndm.ndm_state |= NUD_REACHABLE;
+/* default permanent, same as iproute2's bridge command */
+if (!(ndm.ndm_state & (NUD_PERMANENT | NUD_REACHABLE)))
+ndm.ndm_state |= NUD_PERMANENT;
+
+nl_msg = nlmsg_alloc_simple(isAdd ? RTM_NEWNEIGH : RTM_DELNEIGH,
+NLM_F_REQUEST |
+(isAdd ? (NLM_F_CREATE | NLM_F_EXCL) : 0));
+if (!nl_msg) {
+virReportOOMError();
+return -1;
+}
+
+if (nlmsg_append(nl_msg, &ndm, sizeof(ndm), NLMSG_ALIGNTO) < 0)
+goto buffer_too_small;
+if (nla_put(nl_msg, NDA_LLADDR, VIR_MAC_BUFLEN, mac) < 0)
+goto buffer_too_small;
+
+/* NB: this message can also accept a Destination IP, a port, a
+ * vlan tag, and a via (see iproute2/bridge/fdb.c:fdb_modify()),
+ * but those aren't required for our application
+ */
+
+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 adding fdb entry for %s"), ifname);
+g
[libvirt] [PATCHv3 8/9] qemu: always use virDomainNetGetActualBridgeName to get interface's bridge
qemuNetworkIfaceConnect() used to have a special case for
actualType='network' (a network with forward mode of route, nat, or
isolated) to call the libvirt public API to retrieve the bridge being
used by a network. That is no longer necessary - since all network
types that use a bridge and tap device now get the bridge name stored
in the ActualNetDef, we can just always use
virDomainNetGetActualBridgeName() instead.
(an audit of the two callers to qemuNetworkIfaceConnect() confirms
that it is never called for any other type of network, so the dead
code in the else statement (logging an internal error if it is called
for any other type of network) is eliminated in the process.)
---
No change from V2.
src/qemu/qemu_command.c | 34 +--
src/qemu/qemu_hotplug.c | 54 +++--
2 files changed, 16 insertions(+), 72 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 11ed58b..48bdf4e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -277,6 +277,11 @@ static int
qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr cfg,
return *tapfd < 0 ? -1 : 0;
}
+/* qemuNetworkIfaceConnect - *only* called if actualType is
+ * VIR_DOMAIN_NET_TYPE_NETWORK or VIR_DOMAIN_NET_TYPE_BRIDGE (i.e. if
+ * the connection is made with a tap device connecting to a bridge
+ * device)
+ */
int
qemuNetworkIfaceConnect(virDomainDefPtr def,
virConnectPtr conn,
@@ -286,39 +291,19 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
int *tapfd,
int *tapfdSize)
{
-char *brname = NULL;
+const char *brname;
int ret = -1;
unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
bool template_ifname = false;
-int actualType = virDomainNetGetActualType(net);
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const char *tunpath = "/dev/net/tun";
if (net->backend.tap)
tunpath = net->backend.tap;
-if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
-bool fail = false;
-virNetworkPtr network = virNetworkLookupByName(conn,
- net->data.network.name);
-if (!network)
-return ret;
-
-if (!(brname = virNetworkGetBridgeName(network)))
- fail = true;
-
-virObjectUnref(network);
-if (fail)
-return ret;
-
-} else if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
-if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0)
-return ret;
-} else {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Network type %d is not supported"),
- virDomainNetGetActualType(net));
-return ret;
+if (!(brname = virDomainNetGetActualBridgeName(net))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge name"));
+goto cleanup;
}
if (!net->ifname ||
@@ -400,7 +385,6 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
if (template_ifname)
VIR_FREE(net->ifname);
}
-VIR_FREE(brname);
virObjectUnref(cfg);
return ret;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9467d7d..0d97b57 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1746,58 +1746,20 @@ static virDomainNetDefPtr
*qemuDomainFindNet(virDomainObjPtr vm,
return NULL;
}
-static char *
-qemuDomainNetGetBridgeName(virConnectPtr conn, virDomainNetDefPtr net)
-{
-char *brname = NULL;
-int actualType = virDomainNetGetActualType(net);
-
-if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
-const char *tmpbr = virDomainNetGetActualBridgeName(net);
-if (!tmpbr) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("interface is missing bridge name"));
-goto cleanup;
-}
-/* we need a copy, not just a pointer to the original */
-if (VIR_STRDUP(brname, tmpbr) < 0)
-goto cleanup;
-} else if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
-virNetworkPtr network;
-
-if (!(network = virNetworkLookupByName(conn, net->data.network.name)))
{
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Couldn't find network '%s'"),
- net->data.network.name);
-goto cleanup;
-}
-brname = virNetworkGetBridgeName(network);
-
-virObjectUnref(network);
-} else {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Interface type %d has no bridge name"),
- virDomainNetGetActualType(net));
-}
-
- cleanup:
-return brname;
-}
static int
-qemuDomainChangeNetBridge(virConnectPtr conn,
- virDomainObjPtr vm,
+qemuDomainChangeNetBridge(virDomainObjPtr vm,
[libvirt] [PATCHv3 9/9] lxc: always use virDomainNetGetActualBridgeName to get interface's bridge
lxcProcessSetupInterfaces() used to have a special case for
actualType='network' (a network with forward mode of route, nat, or
isolated) to call the libvirt public API to retrieve the bridge being
used by a network. That is no longer necessary - since all network
types that use a bridge and tap device now get the bridge name stored
in the ActualNetDef, we can just always use
virDomainNetGetActualBridgeName() instead.
---
Change from V2: change attribute name and fix commit log to reference
lxc rather than qemu function.
src/lxc/lxc_driver.c | 26 ++
src/lxc/lxc_process.c | 26 +-
2 files changed, 3 insertions(+), 49 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 97caee3..51c664f 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4161,7 +4161,8 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
actualType = virDomainNetGetActualType(net);
switch (actualType) {
-case VIR_DOMAIN_NET_TYPE_BRIDGE: {
+case VIR_DOMAIN_NET_TYPE_BRIDGE:
+case VIR_DOMAIN_NET_TYPE_NETWORK: {
const char *brname = virDomainNetGetActualBridgeName(net);
if (!brname) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -4174,29 +4175,6 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
brname)))
goto cleanup;
} break;
-case VIR_DOMAIN_NET_TYPE_NETWORK: {
-virNetworkPtr network;
-char *brname = NULL;
-bool fail = false;
-
-if (!(network = virNetworkLookupByName(conn, net->data.network.name)))
-goto cleanup;
-if (!(brname = virNetworkGetBridgeName(network)))
- fail = true;
-
-virObjectUnref(network);
-if (fail)
-goto cleanup;
-
-if (!(veth = virLXCProcessSetupInterfaceBridged(conn,
-vm->def,
-net,
-brname))) {
-VIR_FREE(brname);
-goto cleanup;
-}
-VIR_FREE(brname);
-} break;
case VIR_DOMAIN_NET_TYPE_DIRECT: {
if (!(veth = virLXCProcessSetupInterfaceDirect(conn,
vm->def,
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index de574a9..632fc17 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -382,31 +382,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
type = virDomainNetGetActualType(net);
switch (type) {
-case VIR_DOMAIN_NET_TYPE_NETWORK: {
-virNetworkPtr network;
-char *brname = NULL;
-bool fail = false;
-
-if (!(network = virNetworkLookupByName(conn,
- net->data.network.name)))
-goto cleanup;
-if (!(brname = virNetworkGetBridgeName(network)))
- fail = true;
-
-virObjectUnref(network);
-if (fail)
-goto cleanup;
-
-if (!(veth = virLXCProcessSetupInterfaceBridged(conn,
-def,
-net,
-brname))) {
-VIR_FREE(brname);
-goto cleanup;
-}
-VIR_FREE(brname);
-break;
-}
+case VIR_DOMAIN_NET_TYPE_NETWORK:
case VIR_DOMAIN_NET_TYPE_BRIDGE: {
const char *brname = virDomainNetGetActualBridgeName(net);
if (!brname) {
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 4/9] network: save bridge name in ActualNetDef when actualType==network too
When the actualType of a virDomainNetDef is "network", it means that
we are connecting to a libvirt-managed network (routed, natted, or
isolated) which does use a bridge device (created by libvirt). In the
past we have required drivers such as qemu to call the public API to
retrieve the bridge name in this case (even though it is available in
the NetDef's ActualNetDef if the actualType is "bridge" (i.e., an
externally-created bridge that isn't managed by libvirt). There is no
real reason for this difference, and as a matter of fact it
complicates things for qemu. Also, there is another bridge-related
attribute (macTableManager) that will need to be available in both
cases, so this makes things consistent.
In order to avoid problems when restarting libvirtd after an update
from an older version that *doesn't* store the network's bridgename in
the ActualNetDef, we also need to put it in place during
networkNotifyActualDevice() (this function is run for each interface
of each domain whenever libvirtd is restarted).
Along with making the bridge name available in the internal object, it
is also now reported in the element of the state
XML (or the subelement in the internally-stored format).
The one oddity about this change is that usually there is a separate
union for every different "type" in a higher level object (e.g. in the
case of a virDomainNetDef there are separate "network" and "bridge"
members of the union that pivots on the type), but in this case
network and bridge types both have exactly the same attributes, so the
"bridge" member is used for both type==network and type==bridge.
---
No change from V2.
src/conf/domain_conf.c | 102 +++-
src/network/bridge_driver.c | 20 +
2 files changed, 73 insertions(+), 49 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d81c37..f45969f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1354,6 +1354,7 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
switch (def->type) {
case VIR_DOMAIN_NET_TYPE_BRIDGE:
+case VIR_DOMAIN_NET_TYPE_NETWORK:
VIR_FREE(def->data.bridge.brname);
break;
case VIR_DOMAIN_NET_TYPE_DIRECT:
@@ -7106,9 +7107,12 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
goto error;
}
VIR_FREE(class_id);
-} else if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+}
+if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+actual->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
char *brname = virXPathString("string(./source/@bridge)", ctxt);
-if (!brname) {
+
+if (!brname && actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing element with bridge name in "
"interface's element"));
@@ -17126,60 +17130,59 @@ virDomainHostdevDefFormatCaps(virBufferPtr buf,
static int
virDomainActualNetDefContentsFormat(virBufferPtr buf,
virDomainNetDefPtr def,
-const char *typeStr,
bool inSubelement,
unsigned int flags)
{
-const char *mode;
-
-switch (virDomainNetGetActualType(def)) {
-case VIR_DOMAIN_NET_TYPE_BRIDGE:
-virBufferEscapeString(buf, "\n",
- virDomainNetGetActualBridgeName(def));
-break;
-
-case VIR_DOMAIN_NET_TYPE_DIRECT:
-virBufferAddLit(buf, "\n", mode);
-break;
-
-case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
if (virDomainHostdevDefFormatSubsys(buf,
virDomainNetGetActualHostdev(def),
flags, true) < 0) {
return -1;
}
-break;
-
-case VIR_DOMAIN_NET_TYPE_NETWORK:
-if (!inSubelement) {
-/* the element isn't included in
- * (i.e. when we're putting our output into a subelement
- * rather than inline) because the main element has the
- * same info already. If we're outputting inline, though,
- * we *do* need to output , because the caller
- * won't have done it.
+} else {
+virBufferAddLit(buf, "type == VIR_DOMAIN_NET_TYPE_NETWORK && !inSubelement) {
+/* When we're putting our output into the
+ * subelement rather than the main , the
+ * network name isn't included in the because the
+ * main interface element's has the same info
+ * already. If we've been called to output directly into
+ * the main element's though (the case here -
+ * "!inSubElement"), we *do* need to output the network
+ * name, because the caller won't have done it).
*/
-virBufferEscapeStr
[libvirt] [PATCHv3 5/9] network: store network macTableManager setting in NetDef actual object
At the time that the network driver allocates a connection to a
network, the tap device that will be used hasn't yet been created -
that will be done later by qemu (or lxc or whoever) - but if the
network has macTableManager='libvirt', then when we do get around to
creating the tap device, we will need to add an entry for it to the
network bridge's fdb (forwarding database) *and* turn off learning and
unicast_flood for that tap device in the bridge's sysfs settings. This
means that qemu needs to know both the bridge name as well as the
setting of macTableManager, so we either need to create a new API to
retrieve that info, or just pass it back in the ActualNetDef that is
created during networkAllocateActualDevice. We choose the latter
method, since it's already done for the bridge device, and it has the
side effect of making the information available in domain status.
(NB: in the future, I think that the tap device should actually be
created by networkAllocateActualDevice(), as that will solve several
other problems, but that is a battle for another day, and this
information will still be useful outside the network driver)
---
Change from V2: attribute name.
src/conf/domain_conf.c | 30 ++
src/conf/domain_conf.h | 2 ++
src/libvirt_private.syms| 1 +
src/network/bridge_driver.c | 6 +-
4 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f45969f..843cdec 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -51,6 +51,7 @@
#include "netdev_bandwidth_conf.h"
#include "netdev_vlan_conf.h"
#include "device_conf.h"
+#include "network_conf.h"
#include "virtpm.h"
#include "virstring.h"
@@ -7003,6 +7004,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
char *mode = NULL;
char *addrtype = NULL;
char *trustGuestRxFilters = NULL;
+char *macTableManager = NULL;
if (VIR_ALLOC(actual) < 0)
return -1;
@@ -7119,6 +7121,16 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
goto error;
}
actual->data.bridge.brname = brname;
+macTableManager = virXPathString("string(./source/@macTableManager)",
ctxt);
+if (macTableManager &&
+(actual->data.bridge.macTableManager
+ = virNetworkBridgeMACTableManagerTypeFromString(macTableManager))
<= 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid macTableManager setting '%s' "
+ "in domain interface's element"),
+ macTableManager);
+goto error;
+}
}
bandwidth_node = virXPathNode("./bandwidth", ctxt);
@@ -7139,6 +7151,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
VIR_FREE(mode);
VIR_FREE(addrtype);
VIR_FREE(trustGuestRxFilters);
+VIR_FREE(macTableManager);
virDomainActualNetDefFree(actual);
ctxt->node = save_ctxt;
@@ -17156,12 +17169,18 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
}
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
+int macTableManager =
virDomainNetGetActualBridgeMACTableManager(def);
+
/* actualType == NETWORK includes the name of the bridge
* that is used by the network, whether we are
* "inSubElement" or not.
*/
virBufferEscapeString(buf, " bridge='%s'",
virDomainNetGetActualBridgeName(def));
+if (macTableManager) {
+virBufferAsprintf(buf, " macTableManager='%s'",
+
virNetworkBridgeMACTableManagerTypeToString(macTableManager));
+}
} else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
const char *mode;
@@ -20760,6 +20779,17 @@ virDomainNetGetActualBridgeName(virDomainNetDefPtr
iface)
return NULL;
}
+int
+virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface)
+{
+if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+iface->data.network.actual &&
+(iface->data.network.actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ iface->data.network.actual->type == VIR_DOMAIN_NET_TYPE_NETWORK))
+return iface->data.network.actual->data.bridge.macTableManager;
+return 0;
+}
+
const char *
virDomainNetGetActualDirectDev(virDomainNetDefPtr iface)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 439f3c0..e10b3c5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -888,6 +888,7 @@ struct _virDomainActualNetDef {
union {
struct {
char *brname;
+int macTableManager; /* enum virNetworkBridgeMACTableManagerType */
} bridge;
struct {
char *linkdev;
@@ -2540,6 +2541,7 @@ int
virDomainGraphicsListenSetNetwork(virDomainGraphicsDefPtr def
[libvirt] [PATCHv3 3/9] conf: new network bridge device attribute macTableManager
The macTableManager attribute of a network's bridge subelement tells libvirt how the bridge's MAC address table (used to determine the egress port for packets) is managed. In the default mode, "kernel", management is left to the kernel, which usually determines entries in part by turning on promiscuous mode on all ports of the bridge, flooding packets to all ports when the correct destination is unknown, and adding/removing entries to the fdb as it sees incoming traffic from particular MAC addresses. In "libvirt" mode, libvirt turns off learning and flooding on all the bridge ports connected to guest domain interfaces, and adds/removes entries according to the MAC addresses in the domain interface configurations. A side effect of turning off learning and unicast_flood on the ports of a bridge is that (with Linux kernel 3.17 and newer), the kernel can automatically turn off promiscuous mode on one or more of the bridge's ports (usually only the one interface that is used to connect the bridge to the physical network). The result is better performance (because packets aren't being flooded to all ports, and can be dropped earlier when they are of no interest) and slightly better security (a guest can still send out packets with a spoofed source MAC address, but will only receive traffic intended for the guest interface's configured MAC address). The attribute looks like this in the configuration: test ... This patch only adds the config knob, documentation, and test cases. The functionality behind this knob is added in later patches. --- Change from V2 - changed attribute name from "fdb" to "macTableManager", and values from "learnWithFlooding" and "managed" to "kernel" and "libvirt". docs/formatnetwork.html.in | 50 ++--- docs/schemas/network.rng | 9 src/conf/network_conf.c| 51 +- src/conf/network_conf.h| 11 + src/libvirt_private.syms | 2 + tests/networkxml2xmlin/host-bridge-no-flood.xml| 6 +++ .../nat-network-explicit-flood.xml | 21 + tests/networkxml2xmlout/host-bridge-no-flood.xml | 6 +++ .../nat-network-explicit-flood.xml | 23 ++ tests/networkxml2xmltest.c | 2 + 10 files changed, 164 insertions(+), 17 deletions(-) create mode 100644 tests/networkxml2xmlin/host-bridge-no-flood.xml create mode 100644 tests/networkxml2xmlin/nat-network-explicit-flood.xml create mode 100644 tests/networkxml2xmlout/host-bridge-no-flood.xml create mode 100644 tests/networkxml2xmlout/nat-network-explicit-flood.xml diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 7bcf316..ca0e207 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -81,7 +81,7 @@ ... -+ ... @@ -92,18 +92,56 @@ defines the name of a bridge device which will be used to construct the virtual network. The virtual machines will be connected to this bridge device allowing them to talk to each other. The bridge device -may also be connected to the LAN. It is recommended that bridge -device names started with the prefix vir, but the name -virbr0 is reserved for the "default" virtual -network. This element should always be provided when defining +may also be connected to the LAN. When defining a new network with a mode of + "nat" or "route" (or an isolated network with -no element). +no element), libvirt will +automatically generate a unique name for the bridge device if +none is given, and this name will be permanently stored in the +network configuration so that that the same name will be used +every time the network is started. For these types of networks +(nat, routed, and isolated), a bridge name beginning with the +prefix "virbr" is recommended (and that is what is +auto-generated), but not enforced. Attribute stp specifies if Spanning Tree Protocol is 'on' or 'off' (default is 'on'). Attribute delay sets the bridge's forward delay value in seconds (default is 0). Since 0.3.0 + + + The macTableManager attribute of the bridge + element is used to tell libvirt how the bridge's MAC address + table (used to determine the correct egress port for packets + based on destination MAC address) will be managed. In the + default kernel setting, the kernel + automatically adds and removes entries, typically using +
[libvirt] [PATCHv3 0/9] Let libvirt manage a bridge's MAC table
The idea behind these patches is the following: 1) most virtual machines only have a single MAC address behind each interface, and that MAC address is known by libvirt. 2) If we (i.e. libvirt) manually add an entry to the bridge's forwarding database (fdb) for the MAC address associated with a port on the bridge, we can turn off learning and unicast_flooding for that port. 3) kernels starting with 3.15 (and actually working correctly starting in kernel 3.17) will notice that all of a bridge's ports have flood and learning turned off, and in that case will turn off promiscuous mode on all ports. If all but one of the ports have flood/learning turned off, then promiscuous will be turned off on that port (and left on for all the other ports) 4) When (4) can be done, there is a measurable performance advantage. It can also *kind of* help security, as it will prevent a guest from doing anything useful if it changes its MAC address (but won't prevent the guest from *sending* packets with a spoofed MAC address). NB: These only work with a fixed MAC address, and no vlan tags set in the guest. Support for both of those will be coming. This series is the same as V2, which was previously ACK (pending final determination of attribute name): https://www.redhat.com/archives/libvir-list/2014-December/msg00173.html but with the name of the attribute changed - in V2 it was: fdb="learnWithFlooding|managed" and it is now: macTableManager="kernel|libvirt" which more accurately reflects what is being controlled with the attribute. Laine Stump (9): util: new functions for setting bridge and bridge port attributes util: functions to manage bridge fdb (forwarding database) conf: new network bridge device attribute macTableManager network: save bridge name in ActualNetDef when actualType==network too network: store network macTableManager setting in NetDef actual object network: setup bridge devices for macTableManager='libvirt' qemu: setup tap devices for macTableManager='libvirt' qemu: always use virDomainNetGetActualBridgeName to get interface's bridge lxc: always use virDomainNetGetActualBridgeName to get interface's bridge docs/formatnetwork.html.in | 50 ++- docs/schemas/network.rng | 9 + src/conf/domain_conf.c | 130 --- src/conf/domain_conf.h | 2 + src/conf/network_conf.c| 51 ++- src/conf/network_conf.h| 11 + src/libvirt_private.syms | 11 + src/lxc/lxc_driver.c | 26 +- src/lxc/lxc_process.c | 26 +- src/network/bridge_driver.c| 78 + src/qemu/qemu_command.c| 53 +-- src/qemu/qemu_hotplug.c| 54 +-- src/util/virnetdevbridge.c | 382 - src/util/virnetdevbridge.h | 44 ++- tests/networkxml2xmlin/host-bridge-no-flood.xml| 6 + .../nat-network-explicit-flood.xml | 21 ++ tests/networkxml2xmlout/host-bridge-no-flood.xml | 6 + .../nat-network-explicit-flood.xml | 23 ++ tests/networkxml2xmltest.c | 2 + 19 files changed, 796 insertions(+), 189 deletions(-) create mode 100644 tests/networkxml2xmlin/host-bridge-no-flood.xml create mode 100644 tests/networkxml2xmlin/nat-network-explicit-flood.xml create mode 100644 tests/networkxml2xmlout/host-bridge-no-flood.xml create mode 100644 tests/networkxml2xmlout/nat-network-explicit-flood.xml -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Set path to console on domain startup
On 12/05/2014 05:30 PM, Anthony PERARD wrote: > Hi, > > I'm trying to fix an issue when using OpenStack with libvirt+xen > (libxenlight). > OpenStack cannot access the console output of a Xen PV guest, because the XML > generated by libvirt for a domain is missing the path to the pty. The path > actually appear in the XML once one call virDomainOpenConsole(). > > The patch intend to get the path to the pty without having to call > virDomainOpenConsole, so I've done the work in libxlDomainStart(). So I have a > few question: > Is libxlDomainStart will be called on restore/migrate/reboot? > I guest the function libxlDomainOpenConsole() would not need to do the same > work if the console path is settup properly. > > There is a bug report about this: > https://bugzilla.redhat.com/show_bug.cgi?id=1170743 > Hi, you can leave the bugzilla link in the commit message, if somebody ever needs more context. (And the patch looks good to me, but I'm no libxl expert) Jan > Regards, > > Anthony PERARD (1): > libxl: Set path to console on domain startup. > > src/libxl/libxl_domain.c | 17 + > 1 file changed, 17 insertions(+) > signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 4/4] qemu: add test case for spapr-pci-vfio-host-bridge
On 11/17/2014 11:05 AM, Shivaprasad G Bhat wrote: > The test case adds passthrough hostdevices and interface of > type hostdev. The test case tests the address for multifunction, > multibus, and pci bridges within the spapr-vfio domain. > > Signed-off-by: Shivaprasad G Bhat > Signed-off-by: Pradipta Kumar Banerjee > Reviewed-by: Prerna Saxena > --- > .../qemuxml2argv-hostdev-spapr-vfio.args | 20 + > .../qemuxml2argv-hostdev-spapr-vfio.xml| 75 > > tests/qemuxml2argvtest.c |8 ++ > 3 files changed, 103 insertions(+) > create mode 100644 > tests/qemuxml2argvdata/qemuxml2argv-hostdev-spapr-vfio.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-spapr-vfio.xml This should be squashed into the commit adding the command line generation. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] conf: Rework virDomainObjListFindByUUID to allow more concurrent APIs
On Mon, Dec 08, 2014 at 03:12:11PM +, Daniel P. Berrange wrote: On Mon, Dec 08, 2014 at 04:07:39PM +0100, Peter Krempa wrote: On 12/05/14 15:02, Martin Kletzander wrote: > Currently, when there is an API that's blocking with locked domain and > second API that's waiting in virDomainObjListFindByUUID() for the domain > lock (with the domain list locked) no other API can be executed on any > domain on the whole hypervisor because all would wait for the domain > list to be locked. This patch adds new optional approach to this in > which the domain is only ref'd (reference counter is incremented) > instead of being locked and is locked *after* the list itself is > unlocked. We might consider only ref'ing the domain in the future and > leaving locking on particular APIs, but that's no tonight's fairy tale. > > Signed-off-by: Martin Kletzander > --- > src/conf/domain_conf.c | 27 --- > src/conf/domain_conf.h | 2 ++ > src/libvirt_private.syms | 1 + > 3 files changed, 27 insertions(+), 3 deletions(-) > ACK, although given that the devel freeze is tomorrow and second patch of this series is pretty invasive, I think that giving this change a full dev cycle is a better way to go. Agreed, particularly as this will be a longer dev cycle than usual due to christmas - don't want to risk destablising this release. Definitely, I wouldn't feel very safe this close to release :) I'll push it after 1.2.11 then. Thank you for the review, signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 3/4] qemu: assign addresses for spapr vfio hostdevices and generate cli
On 11/17/2014 11:05 AM, Shivaprasad G Bhat wrote:
> On pseries, the vfio host devices attach to the spapr-pci-vfio domain
> instead of the default emulated domain.
> So, for a host device belonging to iommu group(say) 3, would need below
> host bridge.
> -device spapr-pci-vfio-host-bridge,iommu=3,id=vfiohostbridge3,index=1
>
> The vfio device then needs to assign itself to the bus "vfiohostbridge3"
> as below :
> -device vfio-pci,host=0003:05:00.1,id=hostdev0,bus=vfiohostbridge3.0,\
> addr=0x1
>
> Since each host bridge controller adds a new domain, all the devices
> addressing would need to start from bus0:slot1:function0 in the new domain.
>
> The controller tag for spapr-pci-vfio-host-bridge has the domain and iommu
> number allocated during the parsing based on the hostdevs
> in the xml. Assign the pci addressses for the hostdevs from their
> respective domains.
> The domain id "vfiohostbridge" is used for uniqueness in the
> controller alias.
>
> Signed-off-by: Shivaprasad G Bhat
> Signed-off-by: Pradipta Kumar Banerjee
> Reviewed-by: Prerna Saxena
> ---
> src/conf/domain_addr.c |8 +
> src/conf/domain_addr.h |2
> src/libvirt_private.syms |1
> src/qemu/qemu_command.c | 274
> +++---
> src/qemu/qemu_command.h | 17 +++
> tests/qemuhotplugtest.c |2
> 6 files changed, 282 insertions(+), 22 deletions(-)
>
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index fb4a76f..e6c96f8 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -110,11 +110,11 @@ virDomainPCIAddressValidate(virDomainPCIAddressSetPtr
> addrs,
> virReportError(errType, "%s", _("No PCI buses available"));
> return false;
> }
> -if (addr->domain != 0) {
> +if (addr->domain != addrs->pciDomainId) {
> virReportError(errType,
> _("Invalid PCI address %s. "
> - "Only PCI domain 0 is available"),
> - addrStr);
> + "Only PCI domain %d is available"),
> + addrStr, addrs->pciDomainId);
> return false;
> }
> if (addr->bus >= addrs->nbuses) {
> @@ -463,7 +463,7 @@ virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr
> addrs,
> /* default to starting the search for a free slot from
> * :00:00.0
> */
> -virDevicePCIAddress a = { 0, 0, 0, 0, false };
> +virDevicePCIAddress a = { addrs->pciDomainId, 0, 0, 0, false };
> char *addrStr = NULL;
All the code adding support for non-zero domain to existing code should IMO be
in a separate commit.
>
> /* except if this search is for the exact same type of device as
> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
> index 2c3468e..df4d4e0 100644
> --- a/src/conf/domain_addr.h
> +++ b/src/conf/domain_addr.h
> @@ -64,6 +64,8 @@ struct _virDomainPCIAddressSet {
> size_t nbuses;
> virDevicePCIAddress lastaddr;
> virDomainPCIConnectFlags lastFlags;
> +int pciDomainId; /* The PCI domain to which the devices should
> belong
> + to in the guest */
> bool dryRun; /* on a dry run, new buses are auto-added
> and addresses aren't saved in device infos */
> };
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e6ff977..797fa77 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -213,6 +213,7 @@ virDomainDeviceDefFree;
> virDomainDeviceDefParse;
> virDomainDeviceFindControllerModel;
> virDomainDeviceGetInfo;
> +virDomainDeviceInfoClear;
> virDomainDeviceInfoCopy;
> virDomainDeviceInfoIterate;
> virDomainDeviceTypeToString;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6f10e6d..5813332 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -70,6 +70,8 @@ VIR_LOG_INIT("qemu.qemu_command");
> #define VIO_ADDR_SERIAL 0x3000ul
> #define VIO_ADDR_NVRAM 0x3000ul
>
> +#define DEFAULT_PCI 0
> +
> VIR_ENUM_DECL(virDomainDiskQEMUBus)
> VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST,
>"ide",
> @@ -562,6 +564,12 @@ qemuNetworkPrepareDevices(virDomainDefPtr def)
> goto cleanup;
> if (virDomainDefMaybeAddHostdevSpaprPCIVfioControllers(def) < 0)
> goto cleanup;
> +/* The net device is originally assigned address in generic
> domain.
> + * Clear the original address for the new address to take effect.
> + */
> +if (ARCH_IS_PPC64(def->os.arch) && def->os.machine &&
> +STRPREFIX(def->os.machine, "pseries"))
> +virDomainDeviceInfoClear(&net->info);
> }
> }
> ret = 0;
> @@ -886,6 +894,9 @@ qemuAssignDeviceControllerAlias(virDomainControllerDefPtr
> controller)
> return virAsprintf
Re: [libvirt] [RFC PATCH v2 2/4] qemu: parse spapr-vfio-pci controller from xml
This has a qemu: prefix, even though the parsing is done in conf:
It would look nicer split into several commits:
1) parsing the new controller type (without auto-adding them) + qemuxml2xml
test of the controllers
2) auto-adding the controllers
3) adding the 0 domain parameter to many functions (if needed)
On 11/17/2014 11:04 AM, Shivaprasad G Bhat wrote:
> This patch will get the iommu group for host devices by XML configuration
> of the vfio host bridge controller or from the sysfs.
>
> On pseries, the iommu group can be shared by multiple host devices and
> they would share the same spapr-vfio-host-bus controller. A new
> controller is added for every new iommu group. Every spapr-vfio-host-bridge
> in the cli creates a new pci domain in the guest. For Example,
> -device spapr-pci-vfio-host-bridge,iommu=1,id=SOMEDOMAIN,index=1
> The "SOMEDOMAIN" is the id for new pci domain inside guest.
>
> spapr-pci-vfio-host-bridge is actually a PCI host bridge with VFIO support.
> It hosts pci-bridges, and has all the features/behaviours similar to the
> default emulated pci host bridge. The controller can host both pci and pcie
> devices. For convenience, this root controller uses model "pci-root".
>
> The sample controller tags would look like below:
> iommuGroupNum='3' domain='1'/>
> iommuGroupNum='3' domain='1'>
Would it make sense to fill out the iommuGroup on domain startup, depending on
the devices that were assigned there?
So that libvirt user can do:
Then attach two hostdevs to these domains without knowing their IOMMU group?
(Although the user needs to know which devices are in the same group to attach
more than one to the same domain)
> function='0x0'/>
>
> iommuGroupNum='13' domain='2'/>
>
> Like other architectures, unassigned and unmanaged devices in the iommu group
> need to be detached manually before the guest is created .
>
> The spapr-vfio-pci controllers are removed when there are no corresponding
> hostdevices in xml.
>
> Signed-off-by: Shivaprasad G Bhat
> Signed-off-by: Pradipta Kumar Banerjee
> Reviewed-by: Prerna Saxena
> ---
> docs/schemas/domaincommon.rng | 28 +++
> src/bhyve/bhyve_domain.c |2
> src/conf/domain_conf.c| 165
> +++--
> src/conf/domain_conf.h| 19 +
> src/libvirt_private.syms |1
> src/qemu/qemu_command.c |4 +
> src/qemu/qemu_domain.c| 12 +--
> src/qemu/qemu_driver.c|6 +
> 8 files changed, 222 insertions(+), 15 deletions(-)
>
> @@ -12113,6 +12165,95 @@ virDomainResourceDefParse(xmlNodePtr node,
> return NULL;
> }
>
> +int
> +virDomainDefMaybeAddHostdevSpaprPCIVfioControllers(virDomainDefPtr def)
> +{
> +size_t i, j;
> +virDomainHostdevDefPtr hostdev;
> +virDomainControllerDefPtr controller;
> +int ret = -1;
> +int maxDomainId = 0;
> +int skip;
> +
> +if (!(ARCH_IS_PPC64(def->os.arch)) ||
> +!(def->os.machine && STRPREFIX(def->os.machine, "pseries")))
> +return 0;
> +
> +for (i = 0; i < def->nhostdevs; i++) {
> +hostdev = def->hostdevs[i];
> +if (IS_PCI_VFIO_HOSTDEV(hostdev))
> +hostdev->source.subsys.u.pci.iommu = -1;
> +}
> +/* The hostdevs belonging to same iommu are
> + * all part of same domain.
> + */
> +for (i = 0; i < def->ncontrollers; i++) {
> +controller = def->controllers[i];
> +if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SPAPR_PCI_VFIO &&
> +controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
> +for (j = 0; j < def->nhostdevs; j++) {
> +hostdev = def->hostdevs[j];
> +if (IS_PCI_VFIO_HOSTDEV(hostdev))
> +if (hostdev->info->addr.pci.domain == controller->domain)
> +hostdev->source.subsys.u.pci.iommu =
> controller->opts.spaprvfio.iommuGroupNum;
> +}
> +if (controller->domain > maxDomainId)
> +maxDomainId = controller->domain;
> +}
> +/* If the spapr-vfio controller doesnt exist for the hostdev
> + * add a controller for that iommu group.
> + */
> +for (i = 0; i < def->nhostdevs; i++) {
> +skip = 0;
> +hostdev = def->hostdevs[i];
> +if (IS_PCI_VFIO_HOSTDEV(hostdev)) {
> +virPCIDeviceAddressPtr addr;
> +int iommu = -1;
> +if (hostdev->source.subsys.u.pci.iommu == -1) {
> +addr =
> (virPCIDeviceAddressPtr)&hostdev->source.subsys.u.pci.addr;
> +if ((iommu = virPCIDeviceAddressGetIOMMUGroupNum(addr)) < 0)
> +goto error;
I don't know if we should access /sysfs on XML parsing. Could we add the
controllers at startup instead?
> +hostdev->source.subsys.u.pci.iommu = iommu;
> +
> +for (j = 0; j < def->ncontrollers; j++) {
> +
Re: [libvirt] [RFC PATCH v2 1/4] qemu: Add SPAPR_VFIO_HOST_BRIDGE capability for PPC platform
On 11/17/2014 11:02 AM, Shivaprasad G Bhat wrote:
> To support VFIO for PPC, spapr-vfio-pci-host-bridge is needed in
> QEMU. This patch adds capability for it.
>
> Signed-off-by: Li Zhang
> Signed-off-by: Shivaprasad G Bhat
> Reviewed-by: Prerna Saxena
> ---
> src/qemu/qemu_capabilities.c |2 ++
> src/qemu/qemu_capabilities.h |1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 4bd8a4d..fa80122 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -272,6 +272,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>"migrate-rdma",
>"ivshmem",
>"drive-iotune-max",
> + "spapr-pci-vfio-host-bridge",
> );
>
>
> @@ -1512,6 +1513,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[]
> = {
> { "usb-audio", QEMU_CAPS_OBJECT_USB_AUDIO },
> { "iothread", QEMU_CAPS_OBJECT_IOTHREAD},
> { "ivshmem", QEMU_CAPS_DEVICE_IVSHMEM },
> +{ "spapr-pci-vfio-host-bridge", QEMU_CAPS_SPAPR_VFIO_HOST_BRIDGE },
> };
>
> static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index ffe3494..3f8f8ce 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -219,6 +219,7 @@ typedef enum {
> QEMU_CAPS_MIGRATE_RDMA = 177, /* have rdma migration */
> QEMU_CAPS_DEVICE_IVSHMEM = 178, /* -device ivshmem */
> QEMU_CAPS_DRIVE_IOTUNE_MAX = 179, /* -drive bps_max= and friends */
> +QEMU_CAPS_SPAPR_VFIO_HOST_BRIDGE = 180, /* -device
> spapr-pci-vfio-host-bridge */
>
> QEMU_CAPS_LAST, /* this must always be the last item */
> } virQEMUCapsFlags;
>
It would be nice to test this in qemucapabilities test.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: Set path to console on domain startup.
On Mon, 2014-12-08 at 15:11 +, Anthony PERARD wrote:
> > > The patch intend to get the tty path on the first call of GetXMLDesc.
> >
> > Doesn't it actually do it on domain start (which makes more sense to me
> > anyway).
>
> Just a wording issue. I meant: Have GetXMLDesc always return the path to
> the tty.
>
Ah, yes, I get what you meant now.
> > >
> > > Signed-off-by: Anthony PERARD
> > > ---
> > > src/libxl/libxl_domain.c | 17 +
> > > 1 file changed, 17 insertions(+)
> > >
> > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > > index 9c62291..de56054 100644
> > > --- a/src/libxl/libxl_domain.c
> > > +++ b/src/libxl/libxl_domain.c
> > > @@ -1290,6 +1290,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
> > > virDomainObjPtr vm,
> > > if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> > > goto cleanup_dom;
> > >
> > > +if (vm->def->nconsoles) {
> > > +virDomainChrDefPtr chr = NULL;
> > > +chr = vm->def->consoles[0];
> > > +if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
> > > +libxl_console_type console_type;
> > > +char *console = NULL;
> > > +console_type =
> > > +(chr->targetType ==
> > > VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
> > > + LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
> > > +ret = libxl_console_get_tty(priv->ctx, vm->def->id,
> > > chr->target.port,
> > > +console_type, &console);
> > > +if (!ret)
> > > +ignore_value(VIR_STRDUP(chr->source.data.file.path,
> > > console));
> >
> > libxlDomainOpenConsole will strdup another (well, probably the same)
> > value here, causing a leak I think, so you'll need some check there too
> > I expect.
>
> So, if OpenConsole is call twice, there will also be a leak? Or maybe it
> can not be called twice.
I'm not sure, there may be an existing issue here I suppose.
> Anyway, I though from the use of VIR_STRDUP there that it was safe to
> call VIR_STRDUP several times and it well free the destination. I might
> be wrong.
I wondered about that, but AFAICS VIR_STRDUP is a wrapper around
virStrdup and virStrdup doesn't free any existing destination.
Ian.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] conf: Rework virDomainObjListFindByUUID to allow more concurrent APIs
On Mon, Dec 08, 2014 at 04:07:39PM +0100, Peter Krempa wrote: > On 12/05/14 15:02, Martin Kletzander wrote: > > Currently, when there is an API that's blocking with locked domain and > > second API that's waiting in virDomainObjListFindByUUID() for the domain > > lock (with the domain list locked) no other API can be executed on any > > domain on the whole hypervisor because all would wait for the domain > > list to be locked. This patch adds new optional approach to this in > > which the domain is only ref'd (reference counter is incremented) > > instead of being locked and is locked *after* the list itself is > > unlocked. We might consider only ref'ing the domain in the future and > > leaving locking on particular APIs, but that's no tonight's fairy tale. > > > > Signed-off-by: Martin Kletzander > > --- > > src/conf/domain_conf.c | 27 --- > > src/conf/domain_conf.h | 2 ++ > > src/libvirt_private.syms | 1 + > > 3 files changed, 27 insertions(+), 3 deletions(-) > > > > ACK, although given that the devel freeze is tomorrow and second patch > of this series is pretty invasive, I think that giving this change a > full dev cycle is a better way to go. Agreed, particularly as this will be a longer dev cycle than usual due to christmas - don't want to risk destablising this release. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: Set path to console on domain startup.
On Mon, Dec 08, 2014 at 11:59:44AM +, Ian Campbell wrote:
> On Fri, 2014-12-05 at 16:30 +, Anthony PERARD wrote:
>
> Jim Fehlig maintains the libxl driver in libvirt, so you should CC him
> (I've done so here...)
Thanks.
> > The path to the pty of a Xen PV console is set only in
> > virDomainOpenConsole. But this is done too late. A call to
> > virDomainGetXMLDesc done before OpenConsole will not have the path to
> > the pty, but a call after OpenConsole will.
> >
> > e.g. of the current issue.
> > Starting a domain with ''
> > Then:
> > virDomainGetXMLDesc():
> >
> >
> >
> >
> >
> > virDomainOpenConsole()
> > virDomainGetXMLDesc():
> >
> >
> >
> >
> >
> >
> >
> > The patch intend to get the tty path on the first call of GetXMLDesc.
>
> Doesn't it actually do it on domain start (which makes more sense to me
> anyway).
Just a wording issue. I meant: Have GetXMLDesc always return the path to
the tty.
> >
> > Signed-off-by: Anthony PERARD
> > ---
> > src/libxl/libxl_domain.c | 17 +
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > index 9c62291..de56054 100644
> > --- a/src/libxl/libxl_domain.c
> > +++ b/src/libxl/libxl_domain.c
> > @@ -1290,6 +1290,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
> > virDomainObjPtr vm,
> > if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> > goto cleanup_dom;
> >
> > +if (vm->def->nconsoles) {
> > +virDomainChrDefPtr chr = NULL;
> > +chr = vm->def->consoles[0];
> > +if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
> > +libxl_console_type console_type;
> > +char *console = NULL;
> > +console_type =
> > +(chr->targetType ==
> > VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
> > + LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
> > +ret = libxl_console_get_tty(priv->ctx, vm->def->id,
> > chr->target.port,
> > +console_type, &console);
> > +if (!ret)
> > +ignore_value(VIR_STRDUP(chr->source.data.file.path,
> > console));
>
> libxlDomainOpenConsole will strdup another (well, probably the same)
> value here, causing a leak I think, so you'll need some check there too
> I expect.
So, if OpenConsole is call twice, there will also be a leak? Or maybe it
can not be called twice.
Anyway, I though from the use of VIR_STRDUP there that it was safe to
call VIR_STRDUP several times and it well free the destination. I might
be wrong.
> > +VIR_FREE(console);
> > +}
> > +}
> > +
> > if (!start_paused) {
> > libxl_domain_unpause(priv->ctx, domid);
> > virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
> > VIR_DOMAIN_RUNNING_BOOTED);
--
Anthony PERARD
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] conf: Rework virDomainObjListFindByUUID to allow more concurrent APIs
On 12/05/14 15:02, Martin Kletzander wrote: > Currently, when there is an API that's blocking with locked domain and > second API that's waiting in virDomainObjListFindByUUID() for the domain > lock (with the domain list locked) no other API can be executed on any > domain on the whole hypervisor because all would wait for the domain > list to be locked. This patch adds new optional approach to this in > which the domain is only ref'd (reference counter is incremented) > instead of being locked and is locked *after* the list itself is > unlocked. We might consider only ref'ing the domain in the future and > leaving locking on particular APIs, but that's no tonight's fairy tale. > > Signed-off-by: Martin Kletzander > --- > src/conf/domain_conf.c | 27 --- > src/conf/domain_conf.h | 2 ++ > src/libvirt_private.syms | 1 + > 3 files changed, 27 insertions(+), 3 deletions(-) > ACK, although given that the devel freeze is tomorrow and second patch of this series is pretty invasive, I think that giving this change a full dev cycle is a better way to go. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/12] getstats: start crawling backing chain for qemu
On 12/06/14 09:14, Eric Blake wrote: > Wire up backing chain recursion. Note that for now, we just use > the same allocation numbers for read-only backing files as what > offline domains would report. It is not the correct allocation > number for qcow2 over block devices during block-commit, and it > misses out on the fact that qemu also reports read statistics on > backing files that are worth knowing about (seriously - for a > thin-provisioned setup, it would be nice to easily get at a count > of how many reads were serviced from the backing file in relation > to reads serviced by the active layer). But it is at least > sufficient to prove that the algorithm is working, and to let > other people start coding to the interface while waiting for > later patches that get the correct information. Is that a proof-of-concept then? I'd like to avoid adding stats fields that will change, especially this close to the release as we're running into the risk of baking it in. > > For a running domain, where one of the two images has a backing > file, I see the traditional output: > > $ virsh domstats --block testvm2 > Domain: 'testvm2' > block.count=2 > block.0.name=vda > block.0.path=/tmp/wrapper.qcow2 > block.0.rd.reqs=1 > block.0.rd.bytes=512 > block.0.rd.times=28858 > block.0.wr.reqs=0 > block.0.wr.bytes=0 > block.0.wr.times=0 > block.0.fl.reqs=0 > block.0.fl.times=0 > block.0.allocation=0 > block.0.capacity=131072 > block.0.physical=200704 > block.1.name=vdb > block.1.path=/dev/sda7 > block.1.rd.reqs=0 > block.1.rd.bytes=0 > block.1.rd.times=0 > block.1.wr.reqs=0 > block.1.wr.bytes=0 > block.1.wr.times=0 > block.1.fl.reqs=0 > block.1.fl.times=0 > block.1.allocation=0 > block.1.capacity=131072 > > vs. the new output: > > $ virsh domstats --block --backing testvm2 > Domain: 'testvm2' > block.count=3 > block.0.name=vda > block.0.path=/tmp/wrapper.qcow2 > block.0.rd.reqs=1 > block.0.rd.bytes=512 > block.0.rd.times=28858 > block.0.wr.reqs=0 > block.0.wr.bytes=0 > block.0.wr.times=0 > block.0.fl.reqs=0 > block.0.fl.times=0 > block.0.allocation=0 > block.0.capacity=131072 > block.0.physical=200704 > block.1.name=vda > block.1.path=/dev/sda6 > block.1.backingIndex=1 > block.1.allocation=1073741824 > block.1.capacity=131072 > block.1.physical=1073741824 > block.2.name=vdb > block.2.path=/dev/sda7 > block.2.rd.reqs=0 > block.2.rd.bytes=0 > block.2.rd.times=0 > block.2.wr.reqs=0 > block.2.wr.bytes=0 > block.2.wr.times=0 > block.2.fl.reqs=0 > block.2.fl.times=0 > block.2.allocation=0 > block.2.capacity=131072 ACK to the idea of the stat output idea ... > > * src/qemu/qemu_driver.c (QEMU_DOMAIN_STATS_BACKING): New internal > enum bit. > (qemuConnectGetAllDomainStats): Recognize new user flag, and pass > details to... > (qemuDomainGetStatsBlock): ...here, where we can do longer recursion. > (qemuDomainGetStatsOneBlock): Output new field. > > Signed-off-by: Eric Blake > --- > src/qemu/qemu_driver.c | 55 > +- > 1 file changed, 46 insertions(+), 9 deletions(-) > .. but we shouldn't add code that reports wrong data. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/12] getstats: split block stats reporting for easier recursion
On 12/06/14 09:14, Eric Blake wrote:
> In order to report stats on backing chains, we need to separate
> the output of stats for one block from how we traverse blocks.
>
> * src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Split...
> (qemuDomainGetStatsOneBlock): ...into new helper.
>
> Signed-off-by: Eric Blake
> ---
> src/qemu/qemu_driver.c | 127
> ++---
> 1 file changed, 77 insertions(+), 50 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index cb80f49..feaa4a2 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18529,6 +18529,79 @@ do { \
> goto cleanup; \
> } while (0)
>
> +
> +static int
> +qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
> + virQEMUDriverConfigPtr cfg,
> + virDomainObjPtr dom,
> + virDomainStatsRecordPtr record,
> + int *maxparams,
> + virDomainDiskDefPtr disk,
> + virStorageSourcePtr src,
This is again a bit strange. I think that for other than top-level disks
also should be gathered via monitor. In that case we'll need a way to
'alias' them from qemu's point of view in an uniform way - perhaps -
node names?
> + size_t block_idx,
> + bool abbreviated,
> + virHashTablePtr stats)
> +{
> +qemuBlockStats *entry;
> +int ret = -1;
> +
> +QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx,
> +disk->dst);
> +if (virStorageSourceIsLocalStorage(src) && src->path)
> +QEMU_ADD_NAME_PARAM(record, maxparams, "block", "path",
> +block_idx, src->path);
> +
> +if (abbreviated || !disk->info.alias ||
> +!(entry = virHashLookup(stats, disk->info.alias))) {
> +if (qemuStorageLimitsRefresh(driver, cfg, dom,
> + disk, src, NULL, NULL) < 0)
...
> +goto cleanup;
> +if (src->allocation)
> +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> + "allocation", src->allocation);
> +if (src->capacity)
> +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> + "capacity", src->capacity);
> +if (src->physical)
> +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> + "physical", src->physical);
> +ret = 0;
> +goto cleanup;
> +}
> +
> +QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +"rd.reqs", entry->rd_req);
> +QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +"rd.bytes", entry->rd_bytes);
> +QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +"rd.times", entry->rd_total_times);
> +QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +"wr.reqs", entry->wr_req);
> +QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +"wr.bytes", entry->wr_bytes);
> +QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +"wr.times", entry->wr_total_times);
> +QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +"fl.reqs", entry->flush_req);
> +QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +"fl.times", entry->flush_total_times);
> +
> +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> + "allocation", entry->wr_highest_offset);
> +
> +if (entry->capacity)
> +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> + "capacity", entry->capacity);
> +if (entry->physical)
> +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> + "physical", entry->physical);
> +
> +ret = 0;
> + cleanup:
> +return ret;
> +}
> +
> +
> static int
> qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
> virDomainObjPtr dom,
The recursive approach will be necessary, but I don't really like this
step where we gather the data differently for non-toplevel images.
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/12] getstats: prepare for dynamic block.count stat
On 12/08/14 15:19, Peter Krempa wrote: > On 12/06/14 09:14, Eric Blake wrote: >> A coming patch will make it optionally possible to list backing >> chain block stats; in this mode of operation, block.counts is no >> longer the number of in the domain, but the number of >> blocks in the array being reported. We still want block.count >> listed first, but rather than iterate the tree twice (once to >> count, and once to list stats), it's easier to just touch things >> up after the fact. >> >> * src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Compute count >> after the fact. >> >> Signed-off-by: Eric Blake >> --- >> src/qemu/qemu_driver.c | 9 - >> 1 file changed, 8 insertions(+), 1 deletion(-) >> Compiler complains: qemu/qemu_driver.c: In function 'qemuDomainGetStatsBlock': qemu/qemu_driver.c:18630:46: error: 'i' may be used uninitialized in this function [-Werror=maybe-uninitialized] record->params[count_index].value.ui = i; ^ CC lxc/libvirt_driver_lxc_impl_la-lxc_native.lo CC lxc/libvirt_driver_lxc_impl_la-lxc_driver.lo CC uml/libvirt_driver_uml_impl_la-uml_conf.lo CC uml/libvirt_driver_uml_impl_la-uml_driver.lo CC network/libvirt_driver_network_impl_la-bridge_driver.lo CC network/libvirt_driver_network_impl_la-bridge_driver_platform.lo > > ACK, > if you fix the issue above. > Peter > signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/12] getstats: add new flag for block backing chain
On 12/06/14 09:14, Eric Blake wrote: > This patch introduces access to allocation information about > a backing chain of a live domain. While querying storage > volumes for read-only disks could provide some of the details, > there is one case where we have to rely on qemu: when doing > a block commit into a backing file, where that file is stored > in qcow2 format on a host block device, we want to know the > current highest write offset into that image, in order to know > if the disk must be resized larger. qemu-img does not > (currently) show this information, and none of the earlier > block APIs were extensible enough to expose it. But > virDomainListGetStats is perfect for the job! > > We don't need a new group of statistics, as the existing block > group is sufficient. On the other hand, as existing libvirt > releases already report 1:1 mapping of block.count to > devices, changing the array size could confuse older clients; > and even with newer clients, the time and memory taken to > report additional statistics is not always necessary (backing > files are generally read-only except for block-commit, so while > read statistics may change, sizing statistics will not). So > the choice here is to add a new flag that only newer callers > will pass, when they are prepared for the additional information. > > This patch introduces the new API, but it will take more > patches to get it implemented for qemu. > > * include/libvirt/libvirt-domain.h > (VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING): New flag. > * src/libvirt-domain.c (virConnectGetAllDomainStats): Document it, > and add a new field when it is in use. > * tools/virsh-domain-monitor.c (cmdDomstats): Use new flag. > * tools/virsh.pod (domstats): Document it. > > Signed-off-by: Eric Blake > --- > include/libvirt/libvirt-domain.h | 1 + > src/libvirt-domain.c | 7 ++- > tools/virsh-domain-monitor.c | 7 +++ > tools/virsh.pod | 8 ++-- > 4 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > index cb76d8c..8c4ad7b 100644 > --- a/src/libvirt-domain.c > +++ b/src/libvirt-domain.c > @@ -10903,13 +10903,18 @@ virConnectGetDomainCapabilities(virConnectPtr conn, > * "net..tx.errs" - transmission errors as unsigned long long. > * "net..tx.drop" - transmit packets dropped as unsigned long long. > * > - * VIR_DOMAIN_STATS_BLOCK: Return block devices statistics. > + * VIR_DOMAIN_STATS_BLOCK: Return block devices statistics. By default, > + * this information is limited to the active layer of each of the > + * domain, but adding VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING to @flags > + * will expand the array to cover backing chains. > * The typed parameter keys are in this format: > * "block.count" - number of block devices on this domain > * as unsigned int. > * "block..name" - name of the block device as string. > * matches the target name (vda/sda/hda) of the > * block device. > + * "block..backingIndex" - unsigned int giving the index, > + * when backing images are listed. So name will always be "vda/sda/hda" and this will change according to the position in the backing chain? Okay. That makes sense although it's not entirely obvious from the description. > * "block..path" - string describing the source of block device , > * if it is a file or block device (omitted for network > * sources and drives with no media inserted). ... > diff --git a/tools/virsh.pod b/tools/virsh.pod > index cbd82275..378f1c0 100644 > --- a/tools/virsh.pod > +++ b/tools/virsh.pod > @@ -825,7 +825,7 @@ that require a block device name (such as I or > I for disk snapshots) will accept either target > or unique source names printed by this command. > > -=item B [I<--raw>] [I<--enforce>] [I<--state>] > +=item B [I<--raw>] [I<--enforce>] [I<--backing>] [I<--state>] > [I<--cpu-total>] [I<--balloon>] [I<--vcpu>] [I<--interface>] [I<--block>] > [[I<--list-active>] [I<--list-inactive>] [I<--list-persistent>] > [I<--list-transient>] [I<--list-running>] [I<--list-paused>] > @@ -880,7 +880,11 @@ I<--interface> returns: > "net..tx.errs" - number of transmission errors, > "net..tx.drop" - number of transmit packets dropped > > -I<--block> returns: > +I<--block> returns information about disks associated with each > +domain. Using the I<--backing> flag extends this information to > +cover all resources in the backing chain, rather than the default > +of limiting information to the active layer for each guest disk. > +Information listed includes: > "block.count" - number of block devices on this domain, > "block..name" - name of the target of the block device , > "block..path" - file source of block device , if it is a The man page is missing entry for "block..backingIndex" > ACK if you add the man page entry.
Re: [libvirt] [PATCH 09/12] getstats: prepare for dynamic block.count stat
On 12/06/14 09:14, Eric Blake wrote: > A coming patch will make it optionally possible to list backing > chain block stats; in this mode of operation, block.counts is no > longer the number of in the domain, but the number of > blocks in the array being reported. We still want block.count > listed first, but rather than iterate the tree twice (once to > count, and once to list stats), it's easier to just touch things > up after the fact. > > * src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Compute count > after the fact. > > Signed-off-by: Eric Blake > --- > src/qemu/qemu_driver.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > ACK, Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Waiting for review of [PATCH v2 2/21] hyperv: implementation of virConnectGetVersion
On 25.11.2014 10:10, Yves Vinter wrote: Hi All, As I described the 27^th of October in the following thread: https://www.redhat.com/archives/libvir-list/2014-October/msg00840.html [libvirt] [PATCH v2 0/21] hyperv: hyperv: set of new functionalities new functionalities has been implemented in the libvirt hyperv driver as a set of 21 patches. Can you please rebase and resend? I saw some movement on the thread so I let it go. However, now I realize it was never merged. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/12] getstats: add block.n.path stat
On 12/06/14 09:14, Eric Blake wrote: > I'm about to make block stats optionally more complex to cover > backing chains, where block.count will no longer equal the number > of for a domain. For these reasons, it is nicer if the > statistics output includes the source path (for local files). > This patch doesn't add anything for network disks, although we > may decide to add that later. > > With this patch, I now see the following for the same domain as > in earlier patches (one qcow2 file, and an empty cdrom drive): > $ virsh domstats --block foo > Domain: 'foo' > block.count=2 > block.0.name=hda > block.0.path=/var/lib/libvirt/images/foo.qcow2 > block.0.allocation=200704 > block.0.capacity=42949672960 > block.0.physical=200704 > block.1.name=hdc > > * src/libvirt-domain.c (virConnectGetAllDomainStats): Document > new field. > * tools/virsh.pod (domstats): Document new field. > * src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Return the new > stat for local files/block devices. > (QEMU_ADD_NAME_PARAM): Add parameter. > (qemuDomainGetStatsInterface): Update caller. > > Signed-off-by: Eric Blake > --- > src/libvirt-domain.c | 3 +++ > src/qemu/qemu_driver.c | 11 +++ > tools/virsh.pod| 2 ++ > 3 files changed, 12 insertions(+), 4 deletions(-) > ACK, Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/12] getstats: report block sizes for offline domains
On 12/06/14 09:14, Eric Blake wrote:
> The prior refactoring can now be put to use. With the same domain
> as the previous commit (one qcow2 disk and an empty cdrom drive):
> $ virsh domstats --block foo
> Domain: 'foo'
> block.count=2
> block.0.name=hda
> block.0.allocation=200704
> block.0.capacity=42949672960
> block.0.physical=200704
> block.1.name=hdc
>
> * src/qemu/qemu_driver.c (qemuStorageLimitsRefresh): Tweak
> semantics of helper function.
> (qemuDomainGetStatsBlock): Use it to report offline statistics.
>
> Signed-off-by: Eric Blake
> ---
> src/qemu/qemu_driver.c | 60
> +++---
> 1 file changed, 42 insertions(+), 18 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 404decd..723391b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10983,7 +10983,9 @@ qemuDomainMemoryPeek(virDomainPtr dom,
>
> /* Refresh the capacity and allocation limits of a given storage
> * source. Assumes that the caller has already obtained a domain job.
> - * Set *activeFail to true if data cannot be obtained because a
> + * Pass NULL for path to skip any errors about an empty drive.
> + * Pass NULL for activeFail to skip any monitor attempt, otherwise,
Having a separate offline-only helper in this case would avoid us having
this argument as it would never touch the monitor.
> + * set *activeFail to true if data cannot be obtained because a
> * transient guest is no longer active. */
> static int
> qemuStorageLimitsRefresh(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg,
> @@ -11000,6 +11002,12 @@ qemuStorageLimitsRefresh(virQEMUDriverPtr driver,
> virQEMUDriverConfigPtr cfg,
> char *buf = NULL;
> ssize_t len;
>
> +if (!path) {
> +if (virStorageSourceIsEmpty(src))
> +return 0;
> +path = src->path;
> +}
Again, no need to pass path explicitly.
> +
> /* FIXME: For an offline domain, we always want to check current
> * on-disk statistics (as users have been known to change offline
> * images behind our backs). For a running domain, however, it
...
> @@ -18530,6 +18542,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
> virHashTablePtr stats = NULL;
> qemuDomainObjPrivatePtr priv = dom->privateData;
> bool abbreviated = false;
> +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>
> if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom)) {
> abbreviated = true; /* it's ok, just go ahead silently */
> @@ -18555,8 +18568,18 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
>
> if (abbreviated || !disk->info.alias ||
> !(entry = virHashLookup(stats, disk->info.alias))) {
> -/* FIXME: we could still look up sizing by sharing code
> - * with qemuDomainGetBlockInfo */
> +if (qemuStorageLimitsRefresh(driver, cfg, dom,
> + disk, disk->src, NULL, NULL) < 0)
> +goto cleanup;
> +if (disk->src->allocation)
> +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, i,
> + "allocation",
> disk->src->allocation);
> +if (disk->src->capacity)
> +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, i,
> + "capacity", disk->src->capacity);
> +if (disk->src->physical)
> +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, i,
> + "physical", disk->src->physical);
> continue;
> }
This part looks good. Having stats for offline VMs might be useful.
>
> @@ -18593,6 +18616,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
>
> cleanup:
> virHashFree(stats);
> +virObjectUnref(cfg);
> return ret;
> }
>
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/12] getstats: start giving offline block stats
On 12/06/14 09:14, Eric Blake wrote: > I noticed that for an offline domain, 'virsh domstats --block $dom' > was producing just the domain name, with no stats. But the older > 'virsh domblkinfo' works just fine on offline domains. This patch > starts to get us closer, by at least reporting the disk names for > an offline domain. > > With this patch, I now see the following for an offline domain > with one qcow2 disk and an empty cdrom drive: > $ virsh domstats --block foo > Domain: 'foo' > block.count=2 > block.0.name=hda > block.1.name=hdc > > * src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Don't short-circuit > output of block name. > > Signed-off-by: Eric Blake > --- > src/qemu/qemu_driver.c | 30 +- > 1 file changed, 17 insertions(+), 13 deletions(-) > ACK, Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/12] qemu: refactor blockinfo data gathering
On 12/06/14 09:14, Eric Blake wrote: > Create a helper function that can be reused for gathering block > info from virDomainListGetStats. > > * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Split guts... > (qemuStorageLimitsRefresh): ...into new helper function. > > Signed-off-by: Eric Blake > --- > src/qemu/qemu_driver.c | 196 > ++--- > 1 file changed, 105 insertions(+), 91 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index e873362..1e254bc 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -10981,59 +10981,25 @@ qemuDomainMemoryPeek(virDomainPtr dom, > } > > > +/* Refresh the capacity and allocation limits of a given storage > + * source. Assumes that the caller has already obtained a domain job. > + * Set *activeFail to true if data cannot be obtained because a > + * transient guest is no longer active. */ > static int > -qemuDomainGetBlockInfo(virDomainPtr dom, > - const char *path, > - virDomainBlockInfoPtr info, > - unsigned int flags) > +qemuStorageLimitsRefresh(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg, One argument per line looks better. > + virDomainObjPtr vm, virDomainDiskDefPtr disk, > + virStorageSourcePtr src, const char *path, disk, src and path together? That doesn't seem to make much sense. While 'path' is used for error messages only, and I don't think it's entirely necessary to use the user-provided string in error messages we will be good of with just using src->path. The 'disk' argument is used to gather the allocation stat in case of block devices for an active domain. That is really strange. If you want to make this function usable for any disk source, the disk alias won't be the right way to get that. I think a better approach will be to split the code into two helpers, one being called on offline domains that gathers data from images on disk and doesn't ever touch the monitor (basically the existing code in qemuDomainGetBlockInfo minus the monitor call) and a second helper that will gather the data from monitor calls and never touch files used for online VMs. This is partially available in the existing helper to gather domain block stats. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 3/3] qemu: make persistent update of graphics device supported
We can change vnc password by using virDomainUpdateDeviceFlags API with
live flag. But it can't be changed with config flag. Error is reported as
below.
error: Operation not supported: persistent update of device 'graphics' is not
supported
This patch supports the graphics arguments changed with config flag.
Signed-off-by: Wang Rui
---
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_driver.c | 18 +-
src/qemu/qemu_hotplug.c | 14 ++
src/qemu/qemu_hotplug.h | 2 ++
4 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d81c37..468260c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21066,7 +21066,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
{
virDomainDeviceDefPtr ret = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
-int flags = VIR_DOMAIN_XML_INACTIVE;
+int flags = VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE;
char *xmlStr = NULL;
int rc = -1;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9152cf5..fa03cbe 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7460,6 +7460,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr orig, disk;
+virDomainGraphicsDefPtr newGraphics;
virDomainNetDefPtr net;
int pos;
@@ -7498,6 +7499,22 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
orig->startupPolicy = disk->startupPolicy;
break;
+case VIR_DOMAIN_DEVICE_GRAPHICS:
+newGraphics = dev->data.graphics;
+pos = qemuDomainFindGraphicsIndex(vmdef, newGraphics);
+if (pos < 0) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("cannot find existing graphics type '%s' device
to modify"),
+ virDomainGraphicsTypeToString(newGraphics->type));
+return -1;
+}
+
+virDomainGraphicsDefFree(vmdef->graphics[pos]);
+
+vmdef->graphics[pos] = newGraphics;
+dev->data.graphics = NULL;
+break;
+
case VIR_DOMAIN_DEVICE_NET:
net = dev->data.net;
if ((pos = virDomainNetFindIdx(vmdef, net)) < 0)
@@ -7517,7 +7534,6 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
case VIR_DOMAIN_DEVICE_WATCHDOG:
-case VIR_DOMAIN_DEVICE_GRAPHICS:
case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ec0122b..d7437db 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2312,6 +2312,20 @@ qemuDomainFindGraphics(virDomainObjPtr vm,
}
int
+qemuDomainFindGraphicsIndex(virDomainDefPtr def,
+virDomainGraphicsDefPtr dev)
+{
+size_t i;
+
+for (i = 0; i < def->ngraphics; i++) {
+if (def->graphics[i]->type == dev->type)
+return i;
+}
+
+return -1;
+}
+
+int
qemuDomainChangeGraphics(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainGraphicsDefPtr dev)
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 1c9ca8f..d13c532 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -55,6 +55,8 @@ int qemuDomainAttachHostDevice(virConnectPtr conn,
virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainHostdevDefPtr hostdev);
+int qemuDomainFindGraphicsIndex(virDomainDefPtr def,
+virDomainGraphicsDefPtr dev);
int qemuDomainChangeGraphics(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainGraphicsDefPtr dev);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 1/3] qemu: report properer error number when change graphics failed
It's not supported to change some graphics arguments with '--live'.
Replace some error code VIR_ERR_INTERNAL_ERROR and VIR_ERR_INVALID_ARG
with VIR_ERR_OPERATION_UNSUPPORTED.
Signed-off-by: Wang Rui
---
src/qemu/qemu_hotplug.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9467d7d..d1767bb 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2330,7 +2330,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
}
if (dev->nListens != olddev->nListens) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot change the number of listen addresses"));
goto cleanup;
}
@@ -2340,7 +2340,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
virDomainGraphicsListenDefPtr oldlisten = &olddev->listens[i];
if (newlisten->type != oldlisten->type) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot change the type of listen address"));
goto cleanup;
}
@@ -2348,7 +2348,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
switch ((virDomainGraphicsListenType) newlisten->type) {
case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
if (STRNEQ_NULLABLE(newlisten->address, oldlisten->address)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
dev->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ?
_("cannot change listen address setting on vnc
graphics") :
_("cannot change listen address setting on
spice graphics"));
@@ -2358,7 +2358,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
if (STRNEQ_NULLABLE(newlisten->network, oldlisten->network)) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
dev->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ?
_("cannot change listen network setting on vnc
graphics") :
_("cannot change listen network setting on spice
graphics"));
@@ -2378,12 +2378,12 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
if ((olddev->data.vnc.autoport != dev->data.vnc.autoport) ||
(!dev->data.vnc.autoport &&
(olddev->data.vnc.port != dev->data.vnc.port))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot change port settings on vnc graphics"));
goto cleanup;
}
if (STRNEQ_NULLABLE(olddev->data.vnc.keymap, dev->data.vnc.keymap)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot change keymap setting on vnc graphics"));
goto cleanup;
}
@@ -2424,13 +2424,13 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
(olddev->data.spice.port != dev->data.spice.port)) ||
(!dev->data.spice.autoport &&
(olddev->data.spice.tlsPort != dev->data.spice.tlsPort))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot change port settings on spice graphics"));
goto cleanup;
}
if (STRNEQ_NULLABLE(olddev->data.spice.keymap,
dev->data.spice.keymap)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot change keymap setting on spice
graphics"));
goto cleanup;
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 2/3] qemu: fix alignment of qemuDomainFindGraphics
Signed-off-by: Wang Rui
---
src/qemu/qemu_hotplug.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d1767bb..ec0122b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2297,10 +2297,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
return ret;
}
-
-
-static virDomainGraphicsDefPtr qemuDomainFindGraphics(virDomainObjPtr vm,
- virDomainGraphicsDefPtr
dev)
+static virDomainGraphicsDefPtr
+qemuDomainFindGraphics(virDomainObjPtr vm,
+ virDomainGraphicsDefPtr dev)
{
size_t i;
@@ -2312,7 +2311,6 @@ static virDomainGraphicsDefPtr
qemuDomainFindGraphics(virDomainObjPtr vm,
return NULL;
}
-
int
qemuDomainChangeGraphics(virQEMUDriverPtr driver,
virDomainObjPtr vm,
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 0/3] qemu: support update graphic device persistently
We can change vnc password by using virDomainUpdateDeviceFlags API with live flag. But it can't be changed with config flag. v1: https://www.redhat.com/archives/libvir-list/2014-November/msg00627.html diff to v1: according to Jan's suggestion, 1. (patch 1/3) change error number to VIR_ERR_OPERATION_UNSUPPORTED 2. (patch 3/3) add 'VIR_DOMAIN_XML_SECURE' to flags in initialization. 3. (patch 3/3) Introduce a new function qemuDomainFindGraphicsIndex. Free the old graphics def and replace it with the new one as what we did for DEVICE_NET. Wang Rui (3): qemu: report properer error number when change graphics failed qemu: fix alignment of qemuDomainFindGraphics qemu: make persistent update of graphics device supported src/conf/domain_conf.c | 2 +- src/qemu/qemu_driver.c | 18 +- src/qemu/qemu_hotplug.c | 36 src/qemu/qemu_hotplug.h | 2 ++ 4 files changed, 44 insertions(+), 14 deletions(-) -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: Use gender-neutral pronoun in hacking.html.in
On 12/08/14 14:19, Peter Krempa wrote: > On 12/08/14 14:15, Christophe Fergeau wrote: >> Use 'they' instead of 'he'. >> --- >> docs/hacking.html.in | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> Also, you need to regenerate and push the change to the HACKING file too along with this patch signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: Use gender-neutral pronoun in hacking.html.in
On 12/08/14 14:15, Christophe Fergeau wrote: > Use 'they' instead of 'he'. > --- > docs/hacking.html.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/docs/hacking.html.in b/docs/hacking.html.in > index 32ebd8e..a73b1e0 100644 > --- a/docs/hacking.html.in > +++ b/docs/hacking.html.in > @@ -65,7 +65,7 @@ > review your patch set. One should avoid sending patches as > attachments, > but rather send them in email body along with commit message. If a > developer is sending another version of the patch (e.g. to address > -review comments), he is advised to note differences to previous > +review comments), they are advised to note differences to previous > versions after the --- line in the patch so that it > helps > reviewers but doesn't become part of git history. Moreover, such > patch > needs to be prefixed correctly with > lol, ACK Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: Use gender-neutral pronoun in hacking.html.in
Use 'they' instead of 'he'. --- docs/hacking.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 32ebd8e..a73b1e0 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -65,7 +65,7 @@ review your patch set. One should avoid sending patches as attachments, but rather send them in email body along with commit message. If a developer is sending another version of the patch (e.g. to address -review comments), he is advised to note differences to previous +review comments), they are advised to note differences to previous versions after the --- line in the patch so that it helps reviewers but doesn't become part of git history. Moreover, such patch needs to be prefixed correctly with -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/12] qemu: let blockinfo reuse virStorageSource
On 12/06/14 09:14, Eric Blake wrote:
> Right now, grabbing blockinfo always calls stat on the disk, then
> opens the image to determine the capacity, using a throw-away
> virStorageSourcePtr. This has a couple of drawbacks:
>
> 1. We are calling stat and opening a file on every invocation of
> the API. However, there are cases where the stats should NOT be
> changing between successive calls (if a domain is running, no
> one should be changing the physical size of a block device or raw
> image behind our backs; capacity of read-only files should not
> be changing; and we are the gateway to the block-resize command
> to know when the capacity of read-write files should be changing).
> True, we still have to use stat in some cases (a sparse raw file
> changes allocation if it is read-write and the amount of holes is
> changing, and a read-write qcow2 image stored in a file changes
> physical size if it was not fully pre-allocated). But for
> read-only images, even this should be something we can remember
> from the previous time, rather than repeating every call.
>
> 2. We want to enhance the power of virDomainListGetStats, by
> sharing code. But we already have a virStorageSourcePtr for
> each disk, and it would be easier to reuse the common structure
> than to have to worry about the one-off virDomainBlockInfoPtr.
>
> While this patch does not optimize reuse of information in point
> 1, it does get us closer to being able to do so; by updating a
> structure that survives between consecutive calls.
>
> * src/util/virstoragefile.h (_virStorageSource): Add physical, to
> mirror virDomainBlockInfo.
> * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Store into
> storage source, then copy to block info.
>
> Signed-off-by: Eric Blake
> ---
> src/qemu/qemu_driver.c| 42 ++
> src/util/virstoragefile.h | 3 ++-
> 2 files changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ae4485a..e873362 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11034,6 +11034,26 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
>
> disk = vm->def->disks[idx];
>
> +/* FIXME: For an offline domain, we always want to check current
> + * on-disk statistics (as users have been known to change offline
> + * images behind our backs). For a running domain, however, it
> + * would be nice to avoid opening a file (particularly since
> + * reading a file while qemu is writing it risks the reader seeing
> + * bogus data), or even avoid a stat, if the information
> + * remembered from the prevoius run is still viable.
> + *
> + * For read-only disks, nothing should be changing unless the user
> + * has requested a block-commit action. For read-write disks, we
> + * know some special cases: capacity should not change without a
> + * block-resize (where capacity is the only stat that requires
> + * opening a file, and even then, only for non-raw files); and
> + * physical size of a raw image or of a block device should
> + * likewise not be changing without block-resize. On the other
> + * hand, allocation of a raw file can change (if the file is
> + * sparse, but the amount of sparseness changes due to writes or
> + * punching holes), and physical size of a non-raw file can
> + * change.
For a live VM we should grab all of the above directly from the monitor
and not ever touch the files on the disk. We do that already for the
bulk stats and for getting the right size when doing storage migration.
This function unfortunately is legacy code compared to the stuff I've
pointed out
> + */
> if (virStorageSourceIsLocalStorage(disk->src)) {
> if (!disk->src->path) {
> virReportError(VIR_ERR_INVALID_ARG,
> @@ -11095,15 +5,15 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
> /* Get info for normal formats */
> if (S_ISREG(sb.st_mode) || fd == -1) {
> #ifndef WIN32
> -info->physical = (unsigned long long)sb.st_blocks *
> +disk->src->physical = (unsigned long long)sb.st_blocks *
> (unsigned long long)DEV_BSIZE;
> #else
> -info->physical = sb.st_size;
> +disk->src->physical = sb.st_size;
> #endif
> /* Regular files may be sparse, so logical size (capacity) is not
> same
> * as actual physical above
> */
> -info->capacity = sb.st_size;
> +disk->src->capacity = sb.st_size;
> } else {
> /* NB. Because we configure with AC_SYS_LARGEFILE, off_t should
> * be 64 bits on all platforms.
> @@ -4,17 +11134,17 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
> _("failed to seek to end of %s"), path);
> goto endjob;
> }
> -info->physical = end;
> -info->capacity = end;
> +disk->src->physical = end;
> +disk->sr
[libvirt] [RFC] broken migration by VGA memory patch series
Hi all, The recent patch series that finally start using vram attribute for QEMU video devices and also introduced new vgamem attribute breaks migration from older libvirts to the currently developed version. _Situation before patches:_ The libvirt's XML configuration for VGA device has attribute "vram" but we ignore the value and don't pass anything to QEMU, that sets the VGA ram size to it's default 16MB. _Sitoation after patches:_ The libvirt's XML configuration for VGA device has attribute "vram" and we honor that value and pass it to QEMU, that takes the value and uses it. The migration issue is for the case where you will set with old libvirt different walue than the default 9MB or 16MB in the XML configuration, because the new libvirt will take the 9MB and round it to the next power of 2 which is 16MB. old libvirt guest===> migration ===> new libvirt guest GUEST_1GUEST_1 vram="9182"vram="16384" vgamem_mb=16 vgamem_mb=16 GUEST_2GUEST_2 vram="65536" vram="65536" vgamem_mb=16 vgamem_mb=64 As you can see the migration will fail because we will start using "vram" attribute and because of that the memory for VGA device will be different. The least painful solution of this issue is probable to an extra element in the migration cookie and if this element is missing we will not *pass* the "vram" value to QEMU process so we don't break the migration. This issue also applies to manage-save or save. I would like to get some ideas or comments whether we want to fix the migration from old libvirt or no. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] virsh: Don't reconnect after the command when disconnected
On Fri, Dec 05, 2014 at 12:32:28PM +0100, Jiri Denemark wrote: On Mon, Dec 01, 2014 at 12:00:53 +0100, Martin Kletzander wrote: Each command that needs a connection causes a new connection to be made. Reconnecting after a command failed is pointless, mainly when there is no other command to run. Removeing three lines of code takes care of that and keeps virsh working as it should. Signed-off-by: Martin Kletzander --- tools/virsh.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index bcfa561..0ead9ae 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1958,9 +1958,6 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) if (!ret) vshReportError(ctl); -if (!ret && disconnected != 0) -vshReconnect(ctl); - if (STREQ(cmd->def->name, "quit") || STREQ(cmd->def->name, "exit"))/* hack ... */ return ret; ACK Pushed, thanks. Martin 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/2] rpc: Report proper close reason
On Mon, Dec 01, 2014 at 10:46:38AM +0100, Jiri Denemark wrote:
On Sun, Nov 30, 2014 at 20:09:08 +0100, Martin Kletzander wrote:
Whenever client socket was marked as closed for some reason, it could've
been changed when really closing the connection. With this patch the
proper reason is kept since the first time it's marked as closed.
Signed-off-by: Martin Kletzander
---
src/rpc/virnetclient.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 8657b0e..d7455b5 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -634,8 +634,11 @@ virNetClientMarkClose(virNetClientPtr client,
VIR_DEBUG("client=%p, reason=%d", client, reason);
if (client->sock)
virNetSocketRemoveIOCallback(client->sock);
-client->wantClose = true;
-client->closeReason = reason;
+/* Don't override reason that's already set. */
+if (!client->wantClose) {
+client->wantClose = true;
+client->closeReason = reason;
+}
}
ACK
Pushed, thanks.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: Set path to console on domain startup.
On Fri, 2014-12-05 at 16:30 +, Anthony PERARD wrote:
Jim Fehlig maintains the libxl driver in libvirt, so you should CC him
(I've done so here...)
> The path to the pty of a Xen PV console is set only in
> virDomainOpenConsole. But this is done too late. A call to
> virDomainGetXMLDesc done before OpenConsole will not have the path to
> the pty, but a call after OpenConsole will.
>
> e.g. of the current issue.
> Starting a domain with ''
> Then:
> virDomainGetXMLDesc():
>
>
>
>
>
> virDomainOpenConsole()
> virDomainGetXMLDesc():
>
>
>
>
>
>
>
> The patch intend to get the tty path on the first call of GetXMLDesc.
Doesn't it actually do it on domain start (which makes more sense to me
anyway).
>
> Signed-off-by: Anthony PERARD
> ---
> src/libxl/libxl_domain.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 9c62291..de56054 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -1290,6 +1290,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
> virDomainObjPtr vm,
> if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> goto cleanup_dom;
>
> +if (vm->def->nconsoles) {
> +virDomainChrDefPtr chr = NULL;
> +chr = vm->def->consoles[0];
> +if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
> +libxl_console_type console_type;
> +char *console = NULL;
> +console_type =
> +(chr->targetType ==
> VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
> + LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
> +ret = libxl_console_get_tty(priv->ctx, vm->def->id,
> chr->target.port,
> +console_type, &console);
> +if (!ret)
> +ignore_value(VIR_STRDUP(chr->source.data.file.path,
> console));
libxlDomainOpenConsole will strdup another (well, probably the same)
value here, causing a leak I think, so you'll need some check there too
I expect.
> +VIR_FREE(console);
> +}
> +}
> +
> if (!start_paused) {
> libxl_domain_unpause(priv->ctx, domid);
> virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
> VIR_DOMAIN_RUNNING_BOOTED);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/12] qemu: refactor blockinfo job handling
On 12/06/2014 03:14 AM, Eric Blake wrote:
> In order for a future patch to virDomainListGetStats to reuse
> some code for determining disk usage of offline domains, we
> need to make it easier to pull out part of the guts of grabbing
> blockinfo. The current implementation grabs a job fairly late
> in the game, while getstats will already own a job; reordering
> things so that the job is always grabbed up front in both
> functions will make it easier to pull out the common code.
> This patch results in grabbing a job in cases where one was not
> previously needed, but as it is a query job, it should not be
> noticeably slower.
>
> This patch touches the same code as the fix for CVE-2014-6458
> (commit b799259); in that patch, we avoided hotplug changing
> a disk reference during the time of obtaining a monitor lock
> by copying all data we needed and no longer referencing disk;
> this patch goes the other way and ensures that by holding the
> job, the disk cannot be changed so we no longer need to worry
> about the disk being invalidated across the monitor lock.
>
> * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Rearrange job
> control to be outside of disk information.
>
Ran thru my Coverity checker...
> Signed-off-by: Eric Blake
> ---
> src/qemu/qemu_driver.c | 62
> +++---
> 1 file changed, 29 insertions(+), 33 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a7b208f..ae4485a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10999,7 +10999,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
> int format;
> bool activeFail = false;
> virQEMUDriverConfigPtr cfg = NULL;
> -char *alias = NULL;
> char *buf = NULL;
> ssize_t len;
>
> @@ -11018,11 +11017,19 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
> goto cleanup;
> }
>
> +/* Technically, we only need a job if we are going to query the
> + * monitor, which is only for active domains that are using
> + * non-raw block devices. But it is easier to share code if we
> + * always grab a job; furthermore, grabbing the job ensures that
> + * hot-plug won't change disk behind our backs. */
> +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> +goto cleanup;
> +
> /* Check the path belongs to this domain. */
> if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
> virReportError(VIR_ERR_INVALID_ARG,
> _("invalid path %s not assigned to domain"), path);
> -goto cleanup;
> +goto endjob;
> }
>
> disk = vm->def->disks[idx];
> @@ -11032,36 +11039,36 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
> virReportError(VIR_ERR_INVALID_ARG,
> _("disk '%s' does not currently have a source
> assigned"),
> path);
> -goto cleanup;
> +goto endjob;
> }
>
> if ((fd = qemuOpenFile(driver, vm, disk->src->path, O_RDONLY,
> NULL, NULL)) == -1)
> -goto cleanup;
> +goto endjob;
>
> if (fstat(fd, &sb) < 0) {
> virReportSystemError(errno,
> _("cannot stat file '%s'"),
> disk->src->path);
> -goto cleanup;
> +goto endjob;
> }
>
> if ((len = virFileReadHeaderFD(fd, VIR_STORAGE_MAX_HEADER, &buf)) <
> 0) {
> virReportSystemError(errno, _("cannot read header '%s'"),
> disk->src->path);
> -goto cleanup;
> +goto endjob;
> }
> } else {
> if (virStorageFileInitAs(disk->src, cfg->user, cfg->group) < 0)
> -goto cleanup;
> +goto endjob;
>
> if ((len = virStorageFileReadHeader(disk->src,
> VIR_STORAGE_MAX_HEADER,
> &buf)) < 0)
> -goto cleanup;
> +goto endjob;
>
> if (virStorageFileStat(disk->src, &sb) < 0) {
> virReportSystemError(errno, _("failed to stat remote file '%s'"),
> NULLSTR(disk->src->path));
> -goto cleanup;
> +goto endjob;
> }
> }
>
> @@ -11073,17 +11080,17 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("no disk format for %s and probing is
> disabled"),
> path);
> -goto cleanup;
> +goto endjob;
> }
>
> if ((format = virStorageFileProbeFormatFromBuf(disk->src->path,
> buf, len)) < 0)
> -goto cleanup;
> +goto endjob;
> }
>
> if (!(meta = virStorageFileGetMetadataFromBuf(disk->src->path, buf, len,
>
Re: [libvirt] [PATCH] docs: Correct invalid hyperlinks
On Sat, Dec 06, 2014 at 10:11:27AM +0100, Martin Kletzander wrote: On Fri, Dec 05, 2014 at 09:55:00AM -0700, Eric Blake wrote: On 12/05/2014 03:20 AM, Martin Kletzander wrote: On Fri, Dec 05, 2014 at 09:47:47AM +, Ian Campbell wrote: On Tue, 2014-12-02 at 07:50 +0100, Martin Kletzander wrote: That said, I found out that that XML-XPath is only needed when creating the tarball, but (probably) not when building an RPM with it. It's needed when building from the git tree to, Xen automated testing picked up on this overnight: http://www.chiark.greenend.org.uk/~xensrcts/logs/32083/build-amd64-libvirt/5.ts-libvirt-build.log I'm not sure if things which are only needed to build the tarball are supposed to be checked for by configure or not. We didn't check for XHTML DTDs either, so I don't think so, but OTOH I'm fine with adding that there if majority agrees. Ideally, something that is required for building 'make dist' or 'make rpm' should be required by bootstrap (autogen.sh uses bootstrap.conf to require minimum versions of maintainer-only tools); but if they are optional when building from a tarball, then they should not be required by configure (but configure should still check for them, and we should do a better job of cleanly skipping rather than erroring if an optional build component is not present when building from tarball). How can you add a perl module in "buildreq"? I'm not familiar with how bootstrap works. I had a look at that and posted a patch for gnulib [1] to work with perl modules and additional path for libvirt [2] which takes that into account. Let me see if that works for you. Martin [1] https://lists.gnu.org/archive/html/bug-gnulib/2014-12/msg00108.html [2] https://www.redhat.com/archives/libvir-list/2014-December/msg00399.html signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: Move check for XML::XPath into bootstrap
The module XML::XPath is needed when building from git only (no need to
have it when building from tarball), so this patch moves the check from
specfile into bootstrap.conf.
Signed-off-by: Martin Kletzander
---
This patch needs a gnulib update with this proposed patch in it:
https://lists.gnu.org/archive/html/bug-gnulib/2014-12/msg00108.html
bootstrap.conf | 3 ++-
libvirt.spec.in | 1 -
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index e7ea9c9..c06ee4c 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -1,6 +1,6 @@
# Bootstrap configuration.
-# Copyright (C) 2010-2013 Red Hat, Inc.
+# Copyright (C) 2010-2014 Red Hat, Inc.
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -210,6 +210,7 @@ gzip -
libtool-
patch -
perl 5.5
+perl::XML::XPath -
pkg-config -
python-config -
rpcgen -
diff --git a/libvirt.spec.in b/libvirt.spec.in
index bda28e7..a23aa0a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -427,7 +427,6 @@ BuildRequires: /usr/bin/pod2man
%endif
BuildRequires: git
BuildRequires: perl
-BuildRequires: perl-XML-XPath
BuildRequires: python
%if %{with_systemd}
BuildRequires: systemd-units
--
2.2.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/12] getstats: improve documentation
On 12/06/14 09:14, Eric Blake wrote: > At least with 'virsh domstats --block' on an offline domain, we > currently output no stats even though we recognize the stat > category. Although a later patch will improve this situation, > it is better to document that this is expected behavior. > > Also, while the current implementation rejects filtering flags > for virDomainListGetStats, this limitation may be lifted in the > future and we do not enforce it at the API level. > > * src/libvirt-domain.c (virConnectGetAllDomainStats): Document > that recognized stats might not be reported. > (virDomainListGetStats): Likewise, and tweak filtering documentation. > > Signed-off-by: Eric Blake > --- > src/libvirt-domain.c | 14 +++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > ACK, Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/12] getstats: avoid memory leak on OOM
On 12/06/14 09:14, Eric Blake wrote:
> qemuDomainGetStatsBlock() could leak a stats hash table if it
> encountered OOM while populating the virTypedParameters.
> Oddly, the fix doesn't even touch qemuDomainGetStatsBlock :)
>
> * src/qemu/qemu_driver.c (QEMU_ADD_COUNT_PARAM)
> (QEMU_ADD_NAME_PARAM): Don't return early.
> (qemuDomainGetStatsInterface): Adjust caller.
>
> Signed-off-by: Eric Blake
> ---
> src/qemu/qemu_driver.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9152cf5..a7b208f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18371,7 +18371,7 @@ do { \
>maxparams, \
>param_name, \
>count) < 0) \
> -return -1; \
> +goto cleanup; \
> } while (0)
>
> #define QEMU_ADD_NAME_PARAM(record, maxparams, type, num, name) \
> @@ -18384,7 +18384,7 @@ do { \
> maxparams, \
> param_name, \
> name) < 0) \
> -return -1; \
> +goto cleanup; \
> } while (0)
>
> #define QEMU_ADD_NET_PARAM(record, maxparams, num, name, value) \
> @@ -18448,6 +18448,7 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver
> ATTRIBUTE_UNUSED,
> "tx.drop", tmp.tx_drop);
> }
>
> + cleanup:
With this change qemuDomainGetStatsInterface will return success even in
case when the OOM occurred. You need to add a 'ret' variable set to -1
and set it to 0 just before cleanup, so that the error gets propagated.
> return 0;
> }
>
ACK if you keep reporting the error in OOM case in
qemuDomainGetStatsInterface()
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4] automatic create tap device with network type ethernet
If user not specify network type ethernet, assume that user
needs simple tap device created with libvirt.
This patch does not need to run external script to create tap device or
add root to qemu process. Also libvirt runs script after device creating,
if user provide it.
Difference with v3 that script runs only if it provided.
Signed-off-by: Vasiliy Tolstov
---
src/qemu/qemu_command.c | 119 +++-
src/qemu/qemu_hotplug.c | 10 +---
src/qemu/qemu_process.c | 4 ++
3 files changed, 93 insertions(+), 40 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1831323..78614d5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -276,6 +276,40 @@ static int
qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr cfg,
return *tapfd < 0 ? -1 : 0;
}
+/**
+ * qemuExecuteEthernetScript:
+ * @ifname: the interface name
+ * @script: the script name
+
+ * This function executes script for new tap device created by libvirt.
+ *
+ * Returns 0 in case of success or -1 on failure
+ */
+static int qemuExecuteEthernetScript(const char *ifname, const char *script)
+{
+virCommandPtr cmd;
+int ret;
+
+cmd = virCommandNew(script);
+virCommandAddArgFormat(cmd, "%s", ifname);
+virCommandClearCaps(cmd);
+#ifdef CAP_NET_ADMIN
+virCommandAllowCap(cmd, CAP_NET_ADMIN);
+#endif
+virCommandAddEnvPassCommon(cmd);
+
+if (virCommandRun(cmd, NULL) < 0) {
+ ret = -1;
+} else {
+ ret = 0;
+}
+
+ cleanup:
+virCommandFree(cmd);
+return ret;
+}
+
+
int
qemuNetworkIfaceConnect(virDomainDefPtr def,
virConnectPtr conn,
@@ -313,7 +347,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
} else if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0)
return ret;
-} else {
+} else if (actualType != VIR_DOMAIN_NET_TYPE_ETHERNET) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Network type %d is not supported"),
virDomainNetGetActualType(net));
@@ -335,30 +369,44 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
}
-if (cfg->privileged) {
-if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
- def->uuid, tunpath, tapfd,
*tapfdSize,
-
virDomainNetGetActualVirtPortProfile(net),
- virDomainNetGetActualVlan(net),
- tap_create_flags) < 0) {
+if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
+if (virNetDevTapCreate(&net->ifname, tunpath, tapfd, *tapfdSize,
+ tap_create_flags) < 0) {
virDomainAuditNetDevice(def, net, tunpath, false);
goto cleanup;
}
-} else {
-if (qemuCreateInBridgePortWithHelper(cfg, brname,
- &net->ifname,
- tapfd, tap_create_flags) < 0) {
-virDomainAuditNetDevice(def, net, tunpath, false);
+if (net->script) {
+ if (qemuExecuteEthernetScript(net->ifname, net->script) < 0)
goto cleanup;
}
-/* qemuCreateInBridgePortWithHelper can only create a single FD */
-if (*tapfdSize > 1) {
-VIR_WARN("Ignoring multiqueue network request");
-*tapfdSize = 1;
+if (virNetDevSetOnline(net->ifname, !!(tap_create_flags &
VIR_NETDEV_TAP_CREATE_IFUP)) < 0)
+goto cleanup;
+} else {
+if (cfg->privileged) {
+if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
+ def->uuid, tunpath,
tapfd, *tapfdSize,
+
virDomainNetGetActualVirtPortProfile(net),
+ virDomainNetGetActualVlan(net),
+ tap_create_flags) < 0) {
+virDomainAuditNetDevice(def, net, tunpath, false);
+goto cleanup;
+}
+} else {
+if (qemuCreateInBridgePortWithHelper(cfg, brname,
+ &net->ifname,
+ tapfd,
tap_create_flags) < 0) {
+virDomainAuditNetDevice(def, net, tunpath, false);
+goto cleanup;
+}
+/* qemuCreateInBridgePortWithHelper can only create a single FD */
+if (*tapfdSize > 1) {
+VIR_WARN("Ignoring multiqueue network request");
+*tapfdSize = 1;
+}
}
-}
-virDomainAuditNetDevice(def, net, tunpath, true);
+virDomainAuditNetDevice(def, net, tunpath, true);
+}
if (cfg->macFilter &&
ebtablesAddForwardAllowIn(dri
Re: [libvirt] [PATCH] storage: Fix printing/casting of uid_t/gid_t
On 12/05/14 15:28, John Ferlan wrote:
>
>
> On 12/05/2014 04:16 AM, Peter Krempa wrote:
>> Other parts of libvirt use "%u" for formatting uid/gid and typecast to
>> unsigned int. Storage driver used the signed variant.
>> ---
>> src/storage/storage_driver.c | 21 -
>> 1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
>> index 7f4de19..24b54f4 100644
>> --- a/src/storage/storage_driver.c
>> +++ b/src/storage/storage_driver.c
>> @@ -2797,7 +2797,8 @@ virStorageFileChown(virStorageSourcePtr src,
>> return -2;
>> }
>>
>> -VIR_DEBUG("chown of storage file %p to %d:%d", src, uid, gid);
>> +VIR_DEBUG("chown of storage file %p to %u:%u",
>> + src, (unsigned int)uid, (unsigned int)gid);
>>
>> return src->drv->backend->storageFileChown(src, uid, gid);
>> }
>> @@ -2819,9 +2820,9 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr
>> src,
>> virStorageSourcePtr backingStore = NULL;
>> int backingFormat;
>>
>> -VIR_DEBUG("path=%s format=%d uid=%d gid=%d probe=%d",
>> +VIR_DEBUG("path=%s format=%d uid=%d gid=%u probe=%u",
>
> Need %u for uid and %d for probe (that's a boolean)
Oops. I changed the wrong one.
>
> ACK w/ that
Fixed && pushed.
Thanks.
Peter
>
> John
>
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Waiting for review of [PATCH v2 2/21] hyperv: implementation of virConnectGetVersion
No answer ??? What does it mean : -you don't have a window for the review ? -you are not interested in getting these developments ? -you don't agree with the delivery process I proposed ? -you prefer getting the whole set of the v2 versions before reviewing ? Please, just let me know... Thanks, Yves. De : Yves Vinter Envoyé : mardi 25 novembre 2014 10:10 À : libvir-list@redhat.com Objet : Waiting for review of [libvirt] [PATCH v2 2/21] hyperv: implementation of virConnectGetVersion Hi All, As I described the 27th of October in the following thread: https://www.redhat.com/archives/libvir-list/2014-October/msg00840.html [libvirt] [PATCH v2 0/21] hyperv: hyperv: set of new functionalities new functionalities has been implemented in the libvirt hyperv driver as a set of 21 patches. PATCH 01/21: hyperv: avoid query memleaks on failure PATCH 02/21: hyperv: implementation of virConnectGetVersion PATCH 03/21: hyperv: implementation of virConnectGetCapabilities PATCH 04/21: hyperv: implementation of virDomainGetVcpus and virConnectGetMaxVcpus PATCH 05/21: hyperv: implementation of virNodeGetFreeMemory PATCH 06/21: hyperv: implementation of virDomainShutdown and virDomainShutdownFlags PATCH 07/21: hyperv: implementation of virDomainGetSchedulerType and virDomainGetSchedulerParameters PATCH 08/21: hyperv: implementation of virNetworkLookupByName PATCH 09/21: hyperv: implementation of virNetworkGetXMLDesc PATCH 10/21: hyperv: implementation of virConnectNumOfNetworks and virConnectListNetworks PATCH 11/21: hyperv: implementation of virConnectNumOfDefinedNetworks and virConnectListDefinedNetworks PATCH 12/21: hyperv: implementation of hypervInvokeMethod to handle complex parameters PATCH 13/21: hyperv: implementation of virDomainSetAutostart and virDomainGetAutostart PATCH 14/21: hyperv: implementation of virDomainSetMaxMemory PATCH 15/21: hyperv: implementation of virDomainSetMemory and virDomainSetMemoryFlags PATCH 16/21: hyperv: implementation of virDomainSetVcpus and virDomainSetVcpusFlags PATCH 17/21: hyperv: implementation of virDomainUndefine and virDomainUndefineFlags PATCH 18/21: hyperv: implementation of internal function hypervDomainAttachDisk PATCH 19/21: hyperv: implementation of internal function hypervDomainAttachNetwork PATCH 20/21: hyperv: implementation of virDomainAttachDevice and virDomainAttachDeviceFlags PATCH 21/21: hyperv: implementation of virDomainDefineXML and virDomainCreateXML A first version of this set has been submitted the 8th of October. The first 3 patches have been already reviewed by Eric and the first patch has been pushed in the main stream. Based on the remarks after this 3 reviews, I will produce a V2 for the whole set of patches. It will be more convenient for me to submit V2 versions patch after patch, only after the previous one has been approved and pushed in the main branch. The V2 of patch 02/21 has been submitted the 27th of October here: https://www.redhat.com/archives/libvir-list/2014-October/msg00824.html [libvirt] [PATCH v2 2/21] hyperv: implementation of virConnectGetVersion I've not received any feedback since, and it has not been pushed in the main stream. Is there any issue? I'm still waiting for its approval before submitting the next patch... Thanks for you answer, Yves. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/12] getstats: start crawling backing chain for qemu
On 12/06/14 09:14, Eric Blake wrote:
> Wire up backing chain recursion. Note that for now, we just use
...
>
> Signed-off-by: Eric Blake
> ---
> src/qemu/qemu_driver.c | 55
> +-
> 1 file changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index feaa4a2..b57beeb 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
...
> @@ -18550,8 +18566,16 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
> if (virStorageSourceIsLocalStorage(src) && src->path)
> QEMU_ADD_NAME_PARAM(record, maxparams, "block", "path",
> block_idx, src->path);
> +if (backing_idx)
> +QEMU_ADD_BLOCK_PARAM_UI(record, maxparams, block_idx, "backingIndex",
> +backing_idx);
>
> -if (abbreviated || !disk->info.alias ||
> +/* FIXME: qemu gives information on backing files, but we aren't
> + * currently storing it into the stats table - we need a common
> + * key in qemu_monitor_json.c:qemuMonitorGetAllBlockStatsInfo and
> + * here for getting at that information, probably something like
> + * asprintf("%s.%d", alias, backing_idx). */
Breaks syntax-check:
src/qemu/qemu_driver.c:18577: * asprintf("%s.%d", alias,
backing_idx). */
maint.mk: use virAsprintf, not asprintf
> +if (abbreviated || backing_idx || !disk->info.alias ||
> !(entry = virHashLookup(stats, disk->info.alias))) {
> if (qemuStorageLimitsRefresh(driver, cfg, dom,
> disk, src, NULL, NULL) < 0)
Rest of review will follow in-order.
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/2] qemu: Automaticly create tap device for VIR_DOMAIN_NET_TYPE_ETHERNET
On Fri, Dec 05, 2014 at 11:37:05PM +, Anirban Chakraborty wrote:
>
>
> On 12/5/14, 10:43 AM, "Laine Stump" wrote:
>
> >On 12/05/2014 06:12 AM, Michal Privoznik wrote:
> >> @@ -7374,7 +7399,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
> >> }
> >>
> >> if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> >> -actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> >> +actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> >> +actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
> >> tapfdSize = net->driver.virtio.queues;
> >> if (!tapfdSize)
> >> tapfdSize = 1;
> >
> >So this patch causes libvirt to *always* create a tap device in the
> >traditional manner for type='ethernet'. I wonder if we can safely do
> >this without unknowingly breaking some strange functionality. In
> >particular, what if someone is using type='ethernet' and a script to
> >create some *other* kind of device that outwardly appears to be a tap
> >device, but is created in a different manner - currently you can do this
> >by using type='ethernet' and calling your strange "create my
> >Franken-tap" command from the script. Once this patch is in, you'll no
> >longer be able to do this.
> >
> >(As a matter of fact, I'm getting some sort of phantom memory about
> >someone doing that for some different kind of virtual switch, or
> >possibly some piece of hardware. I can't remember the details though,
> >and it's possible that I'm mistaken - maybe they *were* just creating a
> >standard tap device, but then doing strange things to it.)
>
> In Open Contrail (www.opencontrail.org), we use this feature where tap
> interface is created first, so that we know the name of the tap device a
> priori, before creating the vm. So, this patch will break existing code.
Do you actally pre-create the TAP device though, or merely ask for a
particular choice of name. AFAIK, then using type=ethernet, QEMU
will always attempt to create the TAP device itself, honouring any
name given. Basically whatever QEMU does for type=ethernet, libvirt
should copy in a 100% functionally equivalent manner. We simply want
to move the functionality up a level. So there should be no regressions
if done correctly.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nwfilter: Add support for icmpv6 filtering
On 11/26/2014 10:02 PM, Stefan Berger wrote:
> Make use of the ebtables functionality to be able to filter certain
> parameters of icmpv6 packets. Extend the XML parser for icmpv6 types,
> type ranges, codes, and code ranges. Extend the nwfilter documentation,
> schema, and test cases.
>
> Being able to filter icmpv6 types and codes helps extending the DHCP
> snooper for IPv6 and filtering at least some parameters of IPv6's NDP
> (Neighbor Discovery Protocol) packets. However, the filtering will not
> be as good as the filtering of ARP packets since we cannot check on IP
> addresses in the payload of the NDP packets.
>
> Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
> ---
> docs/formatnwfilter.html.in| 20 +++
> docs/schemas/nwfilter.rng | 26 +
> src/conf/nwfilter_conf.c | 26 +
> src/conf/nwfilter_conf.h | 4 ++
> src/nwfilter/nwfilter_ebiptables_driver.c | 80
> ++
> tests/nwfilterxml2firewalldata/ipv6-linux.args | 16 ++
> tests/nwfilterxml2firewalldata/ipv6.xml| 38
> tests/nwfilterxml2xmlin/ipv6-test.xml | 38
> tests/nwfilterxml2xmlout/ipv6-test.xml | 12
> 9 files changed, 260 insertions(+)
>
> diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in
> index 073b852..7c0dd5b 100644
> diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
> index 074d745..0108dbe 100644
> --- a/src/conf/nwfilter_conf.c
> +++ b/src/conf/nwfilter_conf.c
> @@ -1445,6 +1445,26 @@ static const virXMLAttr2Struct ipv6Attributes[] = {
> .datatype = DATATYPE_UINT16 | DATATYPE_UINT16_HEX,
> .dataIdx = offsetof(virNWFilterRuleDef,
> p.ipv6HdrFilter.portData.dataDstPortEnd),
> },
> +{
> +.name = "type",
> +.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
> +.dataIdx = offsetof(virNWFilterRuleDef,
> p.ipv6HdrFilter.dataICMPTypeStart),
> +},
> +{
> +.name = "typeend",
> +.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
> +.dataIdx = offsetof(virNWFilterRuleDef,
> p.ipv6HdrFilter.dataICMPTypeEnd),
> +},
> +{
> +.name = "code",
> +.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
> +.dataIdx = offsetof(virNWFilterRuleDef,
> p.ipv6HdrFilter.dataICMPCodeStart),
> +},
> +{
> +.name = "codeend",
> +.datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX,
> +.dataIdx = offsetof(virNWFilterRuleDef,
> p.ipv6HdrFilter.dataICMPCodeEnd),
> +},
> COMMENT_PROP_IPHDR(ipv6HdrFilter),
> {
> .name = NULL,
> @@ -2219,6 +2239,12 @@ virNWFilterRuleDefFixup(virNWFilterRuleDefPtr rule)
>rule->p.ipv6HdrFilter.ipHdr.dataSrcIPAddr);
> COPY_NEG_SIGN(rule->p.ipv6HdrFilter.ipHdr.dataDstIPMask,
>rule->p.ipv6HdrFilter.ipHdr.dataDstIPAddr);
> +COPY_NEG_SIGN(rule->p.icmpHdrFilter.dataICMPTypeend,
> + rule->p.icmpHdrFilter.dataICMPType);
> +COPY_NEG_SIGN(rule->p.icmpHdrFilter.dataICMPCode,
> + rule->p.icmpHdrFilter.dataICMPType);
> +COPY_NEG_SIGN(rule->p.icmpHdrFilter.dataICMPCodeend,
> + rule->p.icmpHdrFilter.dataICMPType);
This doesn't compile for me.
> virNWFilterRuleDefFixupIPSet(&rule->p.ipv6HdrFilter.ipHdr);
> break;
>
> diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
> index f81df60..6e68ecc 100644
> --- a/src/conf/nwfilter_conf.h
> +++ b/src/conf/nwfilter_conf.h
> @@ -265,6 +265,10 @@ struct _ipv6HdrFilterDef {
> ethHdrDataDef ethHdr;
> ipHdrDataDef ipHdr;
> portDataDefportData;
> +nwItemDesc dataICMPTypeStart;
> +nwItemDesc dataICMPTypeEnd;
> +nwItemDesc dataICMPCodeStart;
> +nwItemDesc dataICMPCodeEnd;
> };
>
>
> diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
> b/src/nwfilter/nwfilter_ebiptables_driver.c
> index 377b59b..d7a94ee 100644
> --- a/src/nwfilter/nwfilter_ebiptables_driver.c
> +++ b/src/nwfilter/nwfilter_ebiptables_driver.c
> @@ -1826,6 +1826,7 @@ ebtablesCreateRuleInstance(virFirewallPtr fw,
> bool hasMask = false;
> virFirewallRulePtr fwrule;
> int ret = -1;
> +virBuffer buf = VIR_BUFFER_INITIALIZER;
>
> if (STREQ(chainSuffix,
>virNWFilterChainSuffixTypeToString(
> @@ -2342,6 +2343,83 @@ ebtablesCreateRuleInstance(virFirewallPtr fw,
> virFirewallRuleAddArg(fw, fwrule, number);
> }
> }
> +
> +if (HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.dataICMPTypeStart) ||
> +HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.dataICMPTypeEnd) ||
> +HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.dataICMPCodeStart) ||
> +HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.dataICMPCodeEnd) ) {
> +bool lo = false;
> +
