[libvirt] [PATCH 0/4] Misc work on systemd/lxc
This is just a few patches working towards an eventual goal of being ble to tell system the tap/veth devices associated with a qemu/lxc guest. Daniel P. Berrange (4): Add support for systemd-machined CreateMachineWithNetwork Add systemd/dtrace probes for DBus APIs Log dtrace/systemd probes at INFO level instead of DEBUG Change int to size_t in size var for tap/vhost FDs po/POTFILES.in | 1 + src/libvirt_probes.d | 5 ++ src/lxc/lxc_cgroup.c | 1 + src/qemu/qemu_cgroup.c | 1 + src/qemu/qemu_command.c | 18 +++---- src/qemu/qemu_command.h | 10 ++-- src/qemu/qemu_hotplug.c | 4 +- src/util/vircgroup.c | 8 ++++ src/util/vircgroup.h | 2 + src/util/virdbus.c | 26 +++++++++-- src/util/virnetdevtap.c | 8 ++-- src/util/virnetdevtap.h | 4 +- src/util/virprobe.h | 10 ++-- src/util/virsystemd.c | 122 +++++++++++++++++++++++++++++++++++++----------- src/util/virsystemd.h | 2 + tests/virsystemdtest.c | 36 ++++++++++++++ 16 files changed, 202 insertions(+), 56 deletions(-) -- 2.1.0
systemd-machined introduced a new method CreateMachineWithNetwork that obsoletes CreateMachine. It expects to be given a list of VETH/TAP device indexes for the host side device(s) associated with a container/machine. This falls back to the old CreateeMachine method when the new one is not supported. --- po/POTFILES.in | 1 + src/lxc/lxc_cgroup.c | 1 + src/qemu/qemu_cgroup.c | 1 + src/util/vircgroup.c | 8 ++++ src/util/vircgroup.h | 2 + src/util/virsystemd.c | 122 ++++++++++++++++++++++++++++++++++++++----------- src/util/virsystemd.h | 2 + tests/virsystemdtest.c | 36 +++++++++++++++ 8 files changed, 147 insertions(+), 26 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 094c8e3..2db8786 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -216,6 +216,7 @@ src/util/virstorageencryption.c src/util/virstoragefile.c src/util/virstring.c src/util/virsysinfo.c +src/util/virsystemd.c src/util/virerror.c src/util/virerror.h src/util/virtime.c diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index eb67191..728e8e5 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -486,6 +486,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def) NULL, getpid(), true, + 0, NULL, def->resource->partition, -1, &cgroup) < 0) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 1acb77d..d71ffbc 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -769,6 +769,7 @@ qemuInitCgroup(virQEMUDriverPtr driver, NULL, vm->pid, false, + 0, NULL, vm->def->resource->partition, cfg->cgroupControllers, &priv->cgroup) < 0) { diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 64bc647..f5f617e 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1584,6 +1584,8 @@ virCgroupNewMachineSystemd(const char *name, const char *rootdir, pid_t pidleader, bool isContainer, + size_t nnicindexes, + int *nicindexes, const char *partition, int controllers, virCgroupPtr *group) @@ -1602,6 +1604,8 @@ virCgroupNewMachineSystemd(const char *name, rootdir, pidleader, isContainer, + nnicindexes, + nicindexes, partition)) < 0) return rv; @@ -1747,6 +1751,8 @@ virCgroupNewMachine(const char *name, const char *rootdir, pid_t pidleader, bool isContainer, + size_t nnicindexes, + int *nicindexes, const char *partition, int controllers, virCgroupPtr *group) @@ -1762,6 +1768,8 @@ virCgroupNewMachine(const char *name, rootdir, pidleader, isContainer, + nnicindexes, + nicindexes, partition, controllers, group)) == 0) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index f07c1a7..9f984e7 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -100,6 +100,8 @@ int virCgroupNewMachine(const char *name, const char *rootdir, pid_t pidleader, bool isContainer, + size_t nnicindexes, + int *nicindexes, const char *partition, int controllers, virCgroupPtr *group) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index ddfc047..3eea5c2 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -26,6 +26,7 @@ #endif #include "virsystemd.h" +#include "viratomic.h" #include "virdbus.h" #include "virstring.h" #include "viralloc.h" @@ -147,7 +148,10 @@ char *virSystemdMakeMachineName(const char *name, * @uuid: globally unique UUID of the machine * @rootdir: root directory of machine filesystem * @pidleader: PID of the leader process - * @slice: name of the slice to place the machine in + * @iscontainer: true if a container, false if a VM + * @nnicindexes: number of network interface indexes in list + * @nicindexes: list of network interface indexes + * @partition: name of the slice to place the machine in * * Returns 0 on success, -1 on fatal error, or -2 if systemd-machine is not available */ @@ -158,6 +162,8 @@ int virSystemdCreateMachine(const char *name, const char *rootdir, pid_t pidleader, bool iscontainer, + size_t nnicindexes, + int *nicindexes, const char *partition) { int ret; @@ -165,6 +171,7 @@ int virSystemdCreateMachine(const char *name, char *machinename = NULL; char *creatorname = NULL; char *slicename = NULL; + static int hasCreateWithNetwork = 1; ret = virDBusIsServiceEnabled("org.freedesktop.machine1"); if (ret < 0) @@ -192,8 +199,18 @@ int virSystemdCreateMachine(const char *name, } /* - * The systemd DBus API we're invoking has the - * following signature + * The systemd DBus APIs we're invoking have the + * following signature(s) + * + * CreateMachineWithNetwork(in s name, + * in ay id, + * in s service, + * in s class, + * in u leader, + * in s root_directory, + * in ai nicindexes + * in a(sv) scope_properties, + * out o path); * * CreateMachine(in s name, * in ay id, @@ -221,38 +238,91 @@ int virSystemdCreateMachine(const char *name, * @root_directory: the root directory of the container, if * this is known & visible in the host filesystem, or empty string * + * @nicindexes: list of network interface indexes for the + * host end of the VETH device pairs. + * * @scope_properties:an array (not a dict!) of properties that are * passed on to PID 1 when creating a scope unit for your machine. * Will allow initial settings for the cgroup & similar. * * @path: a bus path returned for the machine object created, to * allow further API calls to be made against the object. + * */ VIR_DEBUG("Attempting to create machine via systemd"); - if (virDBusCallMethod(conn, - NULL, - NULL, - "org.freedesktop.machine1", - "/org/freedesktop/machine1", - "org.freedesktop.machine1.Manager", - "CreateMachine", - "sayssusa(sv)", - machinename, - 16, - uuid[0], uuid[1], uuid[2], uuid[3], - uuid[4], uuid[5], uuid[6], uuid[7], - uuid[8], uuid[9], uuid[10], uuid[11], - uuid[12], uuid[13], uuid[14], uuid[15], - creatorname, - iscontainer ? "container" : "vm", - (unsigned int)pidleader, - rootdir ? rootdir : "", - 3, - "Slice", "s", slicename, - "After", "as", 1, "libvirtd.service", - "Before", "as", 1, "libvirt-guests.service") < 0) - goto cleanup; + if (virAtomicIntGet(&hasCreateWithNetwork)) { + DBusError error; + dbus_error_init(&error); + + if (virDBusCallMethod(conn, + NULL, + &error, + "org.freedesktop.machine1", + "/org/freedesktop/machine1", + "org.freedesktop.machine1.Manager", + "CreateMachineWithNetwork", + "sayssusa&ia(sv)", + machinename, + 16, + uuid[0], uuid[1], uuid[2], uuid[3], + uuid[4], uuid[5], uuid[6], uuid[7], + uuid[8], uuid[9], uuid[10], uuid[11], + uuid[12], uuid[13], uuid[14], uuid[15], + creatorname, + iscontainer ? "container" : "vm", + (unsigned int)pidleader, + rootdir ? rootdir : "", + nnicindexes, nicindexes, + 3, + "Slice", "s", slicename, + "After", "as", 1, "libvirtd.service", + "Before", "as", 1, "libvirt-guests.service") < 0) + goto cleanup; + + if (dbus_error_is_set(&error)) { + if (STREQ_NULLABLE("org.freedesktop.DBus.Error.UnknownMethod", + error.name)) { + VIR_INFO("CreateMachineWithNetwork isn't supported, switching " + "to legacy CreateMachine method for systemd-machined"); + dbus_error_free(&error); + virAtomicIntSet(&hasCreateWithNetwork, 0); + /* Could re-structure without Using goto, but this + * avoids another atomic read which would trigger + * another memory barrier */ + goto fallback; + } + virReportError(VIR_ERR_DBUS_SERVICE, + _("CreateMachineWithNetwork: %s"), + error.message ? error.message : _("unknown error")); + goto cleanup; + } + } else { + fallback: + if (virDBusCallMethod(conn, + NULL, + NULL, + "org.freedesktop.machine1", + "/org/freedesktop/machine1", + "org.freedesktop.machine1.Manager", + "CreateMachine", + "sayssusa(sv)", + machinename, + 16, + uuid[0], uuid[1], uuid[2], uuid[3], + uuid[4], uuid[5], uuid[6], uuid[7], + uuid[8], uuid[9], uuid[10], uuid[11], + uuid[12], uuid[13], uuid[14], uuid[15], + creatorname, + iscontainer ? "container" : "vm", + (unsigned int)pidleader, + rootdir ? rootdir : "", + 3, + "Slice", "s", slicename, + "After", "as", 1, "libvirtd.service", + "Before", "as", 1, "libvirt-guests.service") < 0) + goto cleanup; + } ret = 0; diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index 491c9b7..7a29dba 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -40,6 +40,8 @@ int virSystemdCreateMachine(const char *name, const char *rootdir, pid_t pidleader, bool iscontainer, + size_t nnicindexes, + int *nicindexes, const char *partition); int virSystemdTerminateMachine(const char *name, diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 0d57a6a..261c4cc 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -146,6 +146,7 @@ static int testCreateContainer(const void *opaque ATTRIBUTE_UNUSED) "/proc/123/root", 123, true, + 0, NULL, "highpriority.slice") < 0) { fprintf(stderr, "%s", "Failed to create LXC machine\n"); return -1; @@ -181,6 +182,7 @@ static int testCreateMachine(const void *opaque ATTRIBUTE_UNUSED) NULL, 123, false, + 0, NULL, NULL) < 0) { fprintf(stderr, "%s", "Failed to create KVM machine\n"); return -1; @@ -220,6 +222,7 @@ static int testCreateNoSystemd(const void *opaque ATTRIBUTE_UNUSED) NULL, 123, false, + 0, NULL, NULL)) == 0) { unsetenv("FAIL_NO_SERVICE"); fprintf(stderr, "%s", "Unexpected create machine success\n"); @@ -254,6 +257,7 @@ static int testCreateSystemdNotRunning(const void *opaque ATTRIBUTE_UNUSED) NULL, 123, false, + 0, NULL, NULL)) == 0) { unsetenv("FAIL_NOT_REGISTERED"); fprintf(stderr, "%s", "Unexpected create machine success\n"); @@ -288,6 +292,7 @@ static int testCreateBadSystemd(const void *opaque ATTRIBUTE_UNUSED) NULL, 123, false, + 0, NULL, NULL)) == 0) { unsetenv("FAIL_BAD_SERVICE"); fprintf(stderr, "%s", "Unexpected create machine success\n"); @@ -304,6 +309,35 @@ static int testCreateBadSystemd(const void *opaque ATTRIBUTE_UNUSED) } +static int testCreateNetwork(const void *opaque ATTRIBUTE_UNUSED) +{ + unsigned char uuid[VIR_UUID_BUFLEN] = { + 1, 1, 1, 1, + 2, 2, 2, 2, + 3, 3, 3, 3, + 4, 4, 4, 4 + }; + int nicindexes[] = { + 2, 1729, 87539319, + }; + size_t nnicindexes = ARRAY_CARDINALITY(nicindexes); + if (virSystemdCreateMachine("demo", + "lxc", + true, + uuid, + "/proc/123/root", + 123, + true, + nnicindexes, nicindexes, + "highpriority.slice") < 0) { + fprintf(stderr, "%s", "Failed to create LXC machine\n"); + return -1; + } + + return 0; +} + + struct testScopeData { const char *name; const char *partition; @@ -435,6 +469,8 @@ mymain(void) ret = -1; if (virtTestRun("Test create bad systemd ", testCreateBadSystemd, NULL) < 0) ret = -1; + if (virtTestRun("Test create with network ", testCreateNetwork, NULL) < 0) + ret = -1; # define TEST_SCOPE(name, partition, unitname) \ do { \ -- 2.1.0
Hi Daniel, On Wed, 2015-01-14 at 14:05 +0000, Daniel P. Berrange wrote:
systemd-machined introduced a new method CreateMachineWithNetwork that obsoletes CreateMachine. It expects to be given a list of VETH/TAP device indexes for the host side device(s) associated with a container/machine.
This falls back to the old CreateeMachine method when the new one is not supported. --- po/POTFILES.in | 1 + src/lxc/lxc_cgroup.c | 1 + src/qemu/qemu_cgroup.c | 1 + src/util/vircgroup.c | 8 ++++ src/util/vircgroup.h | 2 + src/util/virsystemd.c | 122 ++++++++++++++++++++++++++++++++++++++----------- src/util/virsystemd.h | 2 + tests/virsystemdtest.c | 36 +++++++++++++++ 8 files changed, 147 insertions(+), 26 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in index 094c8e3..2db8786 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -216,6 +216,7 @@ src/util/virstorageencryption.c src/util/virstoragefile.c src/util/virstring.c src/util/virsysinfo.c +src/util/virsystemd.c src/util/virerror.c src/util/virerror.h src/util/virtime.c diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index eb67191..728e8e5 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -486,6 +486,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def) NULL, getpid(), true, + 0, NULL, def->resource->partition, -1, &cgroup) < 0) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 1acb77d..d71ffbc 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -769,6 +769,7 @@ qemuInitCgroup(virQEMUDriverPtr driver, NULL, vm->pid, false, + 0, NULL, vm->def->resource->partition, cfg->cgroupControllers, &priv->cgroup) < 0) { diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 64bc647..f5f617e 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1584,6 +1584,8 @@ virCgroupNewMachineSystemd(const char *name, const char *rootdir, pid_t pidleader, bool isContainer, + size_t nnicindexes, + int *nicindexes, const char *partition, int controllers, virCgroupPtr *group) @@ -1602,6 +1604,8 @@ virCgroupNewMachineSystemd(const char *name, rootdir, pidleader, isContainer, + nnicindexes, + nicindexes, partition)) < 0) return rv;
@@ -1747,6 +1751,8 @@ virCgroupNewMachine(const char *name, const char *rootdir, pid_t pidleader, bool isContainer, + size_t nnicindexes, + int *nicindexes, const char *partition, int controllers, virCgroupPtr *group) @@ -1762,6 +1768,8 @@ virCgroupNewMachine(const char *name, rootdir, pidleader, isContainer, + nnicindexes, + nicindexes, partition, controllers, group)) == 0) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index f07c1a7..9f984e7 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -100,6 +100,8 @@ int virCgroupNewMachine(const char *name, const char *rootdir, pid_t pidleader, bool isContainer, + size_t nnicindexes, + int *nicindexes, const char *partition, int controllers, virCgroupPtr *group) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index ddfc047..3eea5c2 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -26,6 +26,7 @@ #endif
#include "virsystemd.h" +#include "viratomic.h" #include "virdbus.h" #include "virstring.h" #include "viralloc.h" @@ -147,7 +148,10 @@ char *virSystemdMakeMachineName(const char *name, * @uuid: globally unique UUID of the machine * @rootdir: root directory of machine filesystem * @pidleader: PID of the leader process - * @slice: name of the slice to place the machine in + * @iscontainer: true if a container, false if a VM + * @nnicindexes: number of network interface indexes in list + * @nicindexes: list of network interface indexes + * @partition: name of the slice to place the machine in * * Returns 0 on success, -1 on fatal error, or -2 if systemd-machine is not available */ @@ -158,6 +162,8 @@ int virSystemdCreateMachine(const char *name, const char *rootdir, pid_t pidleader, bool iscontainer, + size_t nnicindexes, + int *nicindexes, const char *partition) { int ret; @@ -165,6 +171,7 @@ int virSystemdCreateMachine(const char *name, char *machinename = NULL; char *creatorname = NULL; char *slicename = NULL; + static int hasCreateWithNetwork = 1;
ret = virDBusIsServiceEnabled("org.freedesktop.machine1"); if (ret < 0) @@ -192,8 +199,18 @@ int virSystemdCreateMachine(const char *name, }
/* - * The systemd DBus API we're invoking has the - * following signature + * The systemd DBus APIs we're invoking have the + * following signature(s) + * + * CreateMachineWithNetwork(in s name, + * in ay id, + * in s service, + * in s class, + * in u leader, + * in s root_directory, + * in ai nicindexes + * in a(sv) scope_properties, + * out o path); * * CreateMachine(in s name, * in ay id, @@ -221,38 +238,91 @@ int virSystemdCreateMachine(const char *name, * @root_directory: the root directory of the container, if * this is known & visible in the host filesystem, or empty string * + * @nicindexes: list of network interface indexes for the + * host end of the VETH device pairs. + * * @scope_properties:an array (not a dict!) of properties that are * passed on to PID 1 when creating a scope unit for your machine. * Will allow initial settings for the cgroup & similar. * * @path: a bus path returned for the machine object created, to * allow further API calls to be made against the object. + * */
VIR_DEBUG("Attempting to create machine via systemd"); - if (virDBusCallMethod(conn, - NULL, - NULL, - "org.freedesktop.machine1", - "/org/freedesktop/machine1", - "org.freedesktop.machine1.Manager", - "CreateMachine", - "sayssusa(sv)", - machinename, - 16, - uuid[0], uuid[1], uuid[2], uuid[3], - uuid[4], uuid[5], uuid[6], uuid[7], - uuid[8], uuid[9], uuid[10], uuid[11], - uuid[12], uuid[13], uuid[14], uuid[15], - creatorname, - iscontainer ? "container" : "vm", - (unsigned int)pidleader, - rootdir ? rootdir : "", - 3, - "Slice", "s", slicename, - "After", "as", 1, "libvirtd.service", - "Before", "as", 1, "libvirt-guests.service") < 0) - goto cleanup; + if (virAtomicIntGet(&hasCreateWithNetwork)) { + DBusError error;
If WITH_DBUS was not defined, compilation fault would occur on in here. Regards, Zhu
+ dbus_error_init(&error); + + if (virDBusCallMethod(conn, + NULL, + &error, + "org.freedesktop.machine1", + "/org/freedesktop/machine1", + "org.freedesktop.machine1.Manager", + "CreateMachineWithNetwork", + "sayssusa&ia(sv)", + machinename, + 16, + uuid[0], uuid[1], uuid[2], uuid[3], + uuid[4], uuid[5], uuid[6], uuid[7], + uuid[8], uuid[9], uuid[10], uuid[11], + uuid[12], uuid[13], uuid[14], uuid[15], + creatorname, + iscontainer ? "container" : "vm", + (unsigned int)pidleader, + rootdir ? rootdir : "", + nnicindexes, nicindexes, + 3, + "Slice", "s", slicename, + "After", "as", 1, "libvirtd.service", + "Before", "as", 1, "libvirt-guests.service") < 0) + goto cleanup; + + if (dbus_error_is_set(&error)) { + if (STREQ_NULLABLE("org.freedesktop.DBus.Error.UnknownMethod", + error.name)) { + VIR_INFO("CreateMachineWithNetwork isn't supported, switching " + "to legacy CreateMachine method for systemd-machined"); + dbus_error_free(&error); + virAtomicIntSet(&hasCreateWithNetwork, 0); + /* Could re-structure without Using goto, but this + * avoids another atomic read which would trigger + * another memory barrier */ + goto fallback; + } + virReportError(VIR_ERR_DBUS_SERVICE, + _("CreateMachineWithNetwork: %s"), + error.message ? error.message : _("unknown error")); + goto cleanup; + } + } else { + fallback: + if (virDBusCallMethod(conn, + NULL, + NULL, + "org.freedesktop.machine1", + "/org/freedesktop/machine1", + "org.freedesktop.machine1.Manager", + "CreateMachine", + "sayssusa(sv)", + machinename, + 16, + uuid[0], uuid[1], uuid[2], uuid[3], + uuid[4], uuid[5], uuid[6], uuid[7], + uuid[8], uuid[9], uuid[10], uuid[11], + uuid[12], uuid[13], uuid[14], uuid[15], + creatorname, + iscontainer ? "container" : "vm", + (unsigned int)pidleader, + rootdir ? rootdir : "", + 3, + "Slice", "s", slicename, + "After", "as", 1, "libvirtd.service", + "Before", "as", 1, "libvirt-guests.service") < 0) + goto cleanup; + }
ret = 0;
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index 491c9b7..7a29dba 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -40,6 +40,8 @@ int virSystemdCreateMachine(const char *name, const char *rootdir, pid_t pidleader, bool iscontainer, + size_t nnicindexes, + int *nicindexes, const char *partition);
int virSystemdTerminateMachine(const char *name, diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 0d57a6a..261c4cc 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -146,6 +146,7 @@ static int testCreateContainer(const void *opaque ATTRIBUTE_UNUSED) "/proc/123/root", 123, true, + 0, NULL, "highpriority.slice") < 0) { fprintf(stderr, "%s", "Failed to create LXC machine\n"); return -1; @@ -181,6 +182,7 @@ static int testCreateMachine(const void *opaque ATTRIBUTE_UNUSED) NULL, 123, false, + 0, NULL, NULL) < 0) { fprintf(stderr, "%s", "Failed to create KVM machine\n"); return -1; @@ -220,6 +222,7 @@ static int testCreateNoSystemd(const void *opaque ATTRIBUTE_UNUSED) NULL, 123, false, + 0, NULL, NULL)) == 0) { unsetenv("FAIL_NO_SERVICE"); fprintf(stderr, "%s", "Unexpected create machine success\n"); @@ -254,6 +257,7 @@ static int testCreateSystemdNotRunning(const void *opaque ATTRIBUTE_UNUSED) NULL, 123, false, + 0, NULL, NULL)) == 0) { unsetenv("FAIL_NOT_REGISTERED"); fprintf(stderr, "%s", "Unexpected create machine success\n"); @@ -288,6 +292,7 @@ static int testCreateBadSystemd(const void *opaque ATTRIBUTE_UNUSED) NULL, 123, false, + 0, NULL, NULL)) == 0) { unsetenv("FAIL_BAD_SERVICE"); fprintf(stderr, "%s", "Unexpected create machine success\n"); @@ -304,6 +309,35 @@ static int testCreateBadSystemd(const void *opaque ATTRIBUTE_UNUSED) }
+static int testCreateNetwork(const void *opaque ATTRIBUTE_UNUSED) +{ + unsigned char uuid[VIR_UUID_BUFLEN] = { + 1, 1, 1, 1, + 2, 2, 2, 2, + 3, 3, 3, 3, + 4, 4, 4, 4 + }; + int nicindexes[] = { + 2, 1729, 87539319, + }; + size_t nnicindexes = ARRAY_CARDINALITY(nicindexes); + if (virSystemdCreateMachine("demo", + "lxc", + true, + uuid, + "/proc/123/root", + 123, + true, + nnicindexes, nicindexes, + "highpriority.slice") < 0) { + fprintf(stderr, "%s", "Failed to create LXC machine\n"); + return -1; + } + + return 0; +} + + struct testScopeData { const char *name; const char *partition; @@ -435,6 +469,8 @@ mymain(void) ret = -1; if (virtTestRun("Test create bad systemd ", testCreateBadSystemd, NULL) < 0) ret = -1; + if (virtTestRun("Test create with network ", testCreateNetwork, NULL) < 0) + ret = -1;
# define TEST_SCOPE(name, partition, unitname) \ do { \
On Mon, Jan 19, 2015 at 09:56:35AM +0800, Zhu Guihua wrote:
Hi Daniel,
On Wed, 2015-01-14 at 14:05 +0000, Daniel P. Berrange wrote:
systemd-machined introduced a new method CreateMachineWithNetwork that obsoletes CreateMachine. It expects to be given a list of VETH/TAP device indexes for the host side device(s) associated with a container/machine.
This falls back to the old CreateeMachine method when the new one is not supported. --- po/POTFILES.in | 1 + src/lxc/lxc_cgroup.c | 1 + src/qemu/qemu_cgroup.c | 1 + src/util/vircgroup.c | 8 ++++ src/util/vircgroup.h | 2 + src/util/virsystemd.c | 122 ++++++++++++++++++++++++++++++++++++++----------- src/util/virsystemd.h | 2 + tests/virsystemdtest.c | 36 +++++++++++++++ 8 files changed, 147 insertions(+), 26 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in index 094c8e3..2db8786 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -216,6 +216,7 @@ src/util/virstorageencryption.c src/util/virstoragefile.c src/util/virstring.c src/util/virsysinfo.c +src/util/virsystemd.c src/util/virerror.c src/util/virerror.h src/util/virtime.c diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index eb67191..728e8e5 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -486,6 +486,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def) NULL, getpid(), true, + 0, NULL, def->resource->partition, -1, &cgroup) < 0) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 1acb77d..d71ffbc 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -769,6 +769,7 @@ qemuInitCgroup(virQEMUDriverPtr driver, NULL, vm->pid, false, + 0, NULL, vm->def->resource->partition, cfg->cgroupControllers, &priv->cgroup) < 0) { diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 64bc647..f5f617e 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1584,6 +1584,8 @@ virCgroupNewMachineSystemd(const char *name, const char *rootdir, pid_t pidleader, bool isContainer, + size_t nnicindexes, + int *nicindexes, const char *partition, int controllers, virCgroupPtr *group) @@ -1602,6 +1604,8 @@ virCgroupNewMachineSystemd(const char *name, rootdir, pidleader, isContainer, + nnicindexes, + nicindexes, partition)) < 0) return rv;
@@ -1747,6 +1751,8 @@ virCgroupNewMachine(const char *name, const char *rootdir, pid_t pidleader, bool isContainer, + size_t nnicindexes, + int *nicindexes, const char *partition, int controllers, virCgroupPtr *group) @@ -1762,6 +1768,8 @@ virCgroupNewMachine(const char *name, rootdir, pidleader, isContainer, + nnicindexes, + nicindexes, partition, controllers, group)) == 0) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index f07c1a7..9f984e7 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -100,6 +100,8 @@ int virCgroupNewMachine(const char *name, const char *rootdir, pid_t pidleader, bool isContainer, + size_t nnicindexes, + int *nicindexes, const char *partition, int controllers, virCgroupPtr *group) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index ddfc047..3eea5c2 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -26,6 +26,7 @@ #endif
#include "virsystemd.h" +#include "viratomic.h" #include "virdbus.h" #include "virstring.h" #include "viralloc.h" @@ -147,7 +148,10 @@ char *virSystemdMakeMachineName(const char *name, * @uuid: globally unique UUID of the machine * @rootdir: root directory of machine filesystem * @pidleader: PID of the leader process - * @slice: name of the slice to place the machine in + * @iscontainer: true if a container, false if a VM + * @nnicindexes: number of network interface indexes in list + * @nicindexes: list of network interface indexes + * @partition: name of the slice to place the machine in * * Returns 0 on success, -1 on fatal error, or -2 if systemd-machine is not available */ @@ -158,6 +162,8 @@ int virSystemdCreateMachine(const char *name, const char *rootdir, pid_t pidleader, bool iscontainer, + size_t nnicindexes, + int *nicindexes, const char *partition) { int ret; @@ -165,6 +171,7 @@ int virSystemdCreateMachine(const char *name, char *machinename = NULL; char *creatorname = NULL; char *slicename = NULL; + static int hasCreateWithNetwork = 1;
ret = virDBusIsServiceEnabled("org.freedesktop.machine1"); if (ret < 0) @@ -192,8 +199,18 @@ int virSystemdCreateMachine(const char *name, }
/* - * The systemd DBus API we're invoking has the - * following signature + * The systemd DBus APIs we're invoking have the + * following signature(s) + * + * CreateMachineWithNetwork(in s name, + * in ay id, + * in s service, + * in s class, + * in u leader, + * in s root_directory, + * in ai nicindexes + * in a(sv) scope_properties, + * out o path); * * CreateMachine(in s name, * in ay id, @@ -221,38 +238,91 @@ int virSystemdCreateMachine(const char *name, * @root_directory: the root directory of the container, if * this is known & visible in the host filesystem, or empty string * + * @nicindexes: list of network interface indexes for the + * host end of the VETH device pairs. + * * @scope_properties:an array (not a dict!) of properties that are * passed on to PID 1 when creating a scope unit for your machine. * Will allow initial settings for the cgroup & similar. * * @path: a bus path returned for the machine object created, to * allow further API calls to be made against the object. + * */
VIR_DEBUG("Attempting to create machine via systemd"); - if (virDBusCallMethod(conn, - NULL, - NULL, - "org.freedesktop.machine1", - "/org/freedesktop/machine1", - "org.freedesktop.machine1.Manager", - "CreateMachine", - "sayssusa(sv)", - machinename, - 16, - uuid[0], uuid[1], uuid[2], uuid[3], - uuid[4], uuid[5], uuid[6], uuid[7], - uuid[8], uuid[9], uuid[10], uuid[11], - uuid[12], uuid[13], uuid[14], uuid[15], - creatorname, - iscontainer ? "container" : "vm", - (unsigned int)pidleader, - rootdir ? rootdir : "", - 3, - "Slice", "s", slicename, - "After", "as", 1, "libvirtd.service", - "Before", "as", 1, "libvirt-guests.service") < 0) - goto cleanup; + if (virAtomicIntGet(&hasCreateWithNetwork)) { + DBusError error;
If WITH_DBUS was not defined, compilation fault would occur on in here.
Thanks for mentioning - i'll investigate. 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 :|
When debugging libvirt it is helpful to set probes around RPC calls. We already have probes for libvirt's native RPC layer, so it makes sense to add them for the DBus RPC layer too. --- src/libvirt_probes.d | 5 +++++ src/util/virdbus.c | 26 +++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/libvirt_probes.d b/src/libvirt_probes.d index 340d665..c5bda5c 100644 --- a/src/libvirt_probes.d +++ b/src/libvirt_probes.d @@ -15,6 +15,11 @@ provider libvirt { probe event_poll_run(int nfds, int timeout); + # file: src/util/virdbus.c + # prefix: dbus + probe dbus_method_call(const char *interface, const char *member, const char *object, const char *destination); + probe dbus_method_error(const char *interface, const char *member, const char *object, const char *destination, const char *name, const char *message); + probe dbus_method_reply(const char *interface, const char *member, const char *object, const char *destination); # file: src/util/virobject.c # prefix: object diff --git a/src/util/virdbus.c b/src/util/virdbus.c index ee8732c..d9665c1 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -27,6 +27,7 @@ #include "virlog.h" #include "virthread.h" #include "virstring.h" +#include "virprobe.h" #define VIR_FROM_THIS VIR_FROM_DBUS @@ -1521,20 +1522,35 @@ static int virDBusCall(DBusConnection *conn, DBusMessage *call, DBusMessage **replyout, - DBusError *error, - const char *member) + DBusError *error) + { DBusMessage *reply = NULL; DBusError localerror; int ret = -1; + const char *iface, *member, *path, *dest; if (!error) dbus_error_init(&localerror); + iface = dbus_message_get_interface(call); + member = dbus_message_get_member(call); + path = dbus_message_get_path(call); + dest = dbus_message_get_destination(call); + + PROBE(DBUS_METHOD_CALL, + "'%s.%s' on '%s' at '%s'", + iface, member, path, dest); + if (!(reply = dbus_connection_send_with_reply_and_block(conn, call, VIR_DBUS_METHOD_CALL_TIMEOUT_MILLIS, error ? error : &localerror))) { + PROBE(DBUS_METHOD_ERROR, + "'%s.%s' on '%s' at '%s' error %s: %s", + iface, member, path, dest, + error ? error->name : localerror.name, + error ? error->message : localerror.message); if (error) { ret = 0; } else { @@ -1544,6 +1560,10 @@ virDBusCall(DBusConnection *conn, goto cleanup; } + PROBE(DBUS_METHOD_REPLY, + "'%s.%s' on '%s' at '%s'", + iface, member, path, dest); + ret = 0; cleanup: @@ -1616,7 +1636,7 @@ int virDBusCallMethod(DBusConnection *conn, ret = -1; - ret = virDBusCall(conn, call, replyout, error, member); + ret = virDBusCall(conn, call, replyout, error); cleanup: if (call) -- 2.1.0
Every dtrace/systemd probe also include a libvirt log message. These are logged at level DEBUG currently, which means if you want to see all probes they are drowned by the rest of the DEBUG messages. Since we don't really use the INFO log level for much, it seems reasonable to suggest we log all probes at level INFO. --- src/util/virprobe.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/util/virprobe.h b/src/util/virprobe.h index fe3c422..7565954 100644 --- a/src/util/virprobe.h +++ b/src/util/virprobe.h @@ -83,8 +83,8 @@ # define PROBE_EXPAND(NAME, ARGS) NAME(ARGS) # define PROBE(NAME, FMT, ...) \ - VIR_DEBUG_INT(&virLogSelf, \ - NULL, __LINE__, __func__, \ + VIR_INFO_INT(&virLogSelf, \ + __FILE__, __LINE__, __func__, \ #NAME ": " FMT, __VA_ARGS__); \ if (LIBVIRT_ ## NAME ## _ENABLED()) { \ PROBE_EXPAND(LIBVIRT_ ## NAME, \ @@ -92,9 +92,9 @@ } # else # define PROBE(NAME, FMT, ...) \ - VIR_DEBUG_INT(&virLogSelf, \ - NULL, __LINE__, __func__, \ - #NAME ": " FMT, __VA_ARGS__); + VIR_INFO_INT(&virLogSelf, \ + __FILE__, __LINE__, __func__, \ + #NAME ": " FMT, __VA_ARGS__); # endif #endif /* __VIR_PROBE_H__ */ -- 2.1.0
A number of methods take an int for a parameter that indicates the size of an array. The correct type for array sizes is size_t --- src/qemu/qemu_command.c | 18 +++++++++--------- src/qemu/qemu_command.h | 10 +++++----- src/qemu/qemu_hotplug.c | 4 ++-- src/util/virnetdevtap.c | 8 ++++---- src/util/virnetdevtap.h | 4 ++-- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1930abd..99edf45 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -289,7 +289,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, int *tapfd, - int *tapfdSize) + size_t *tapfdSize) { const char *brname; int ret = -1; @@ -433,7 +433,7 @@ qemuOpenVhostNet(virDomainDefPtr def, virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, int *vhostfd, - int *vhostfdSize) + size_t *vhostfdSize) { size_t i; const char *vhostnet_path = net->backend.vhost; @@ -490,7 +490,7 @@ qemuOpenVhostNet(virDomainDefPtr def, "but is unavailable")); goto error; } - VIR_WARN("Unable to open vhost-net. Opened so far %zu, requested %d", + VIR_WARN("Unable to open vhost-net. Opened so far %zu, requested %zu", i, *vhostfdSize); *vhostfdSize = i; break; @@ -4353,7 +4353,7 @@ qemuBuildNicDevStr(virDomainDefPtr def, virDomainNetDefPtr net, int vlan, int bootindex, - int vhostfdSize, + size_t vhostfdSize, virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4455,7 +4455,7 @@ qemuBuildNicDevStr(virDomainDefPtr def, if (usingVirtio && vhostfdSize > 1) { /* As advised at http://www.linux-kvm.org/page/Multiqueue * we should add vectors=2*N+2 where N is the vhostfdSize */ - virBufferAsprintf(&buf, ",mq=on,vectors=%d", 2 * vhostfdSize + 2); + virBufferAsprintf(&buf, ",mq=on,vectors=%zu", 2 * vhostfdSize + 2); } if (vlan == -1) virBufferAsprintf(&buf, ",netdev=host%s", net->info.alias); @@ -4488,9 +4488,9 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, char type_sep, int vlan, char **tapfd, - int tapfdSize, + size_t tapfdSize, char **vhostfd, - int vhostfdSize) + size_t vhostfdSize) { bool is_tap = false; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -7352,9 +7352,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, int ret = -1; char *nic = NULL, *host = NULL; int *tapfd = NULL; - int tapfdSize = 0; + size_t tapfdSize = 0; int *vhostfd = NULL; - int vhostfdSize = 0; + size_t vhostfdSize = 0; char **tapfdName = NULL; char **vhostfdName = NULL; int actualType = virDomainNetGetActualType(net); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index f7d3c2d..d8adf59 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -96,9 +96,9 @@ char *qemuBuildHostNetStr(virDomainNetDefPtr net, char type_sep, int vlan, char **tapfd, - int tapfdSize, + size_t tapfdSize, char **vhostfd, - int vhostfdSize); + size_t vhostfdSize); /* Legacy, pre device support */ char *qemuBuildNicStr(virDomainNetDefPtr net, @@ -110,7 +110,7 @@ char *qemuBuildNicDevStr(virDomainDefPtr def, virDomainNetDefPtr net, int vlan, int bootindex, - int vhostfdSize, + size_t vhostfdSize, virQEMUCapsPtr qemuCaps); char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, @@ -193,7 +193,7 @@ int qemuNetworkIfaceConnect(virDomainDefPtr def, virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, int *tapfd, - int *tapfdSize) + size_t *tapfdSize) ATTRIBUTE_NONNULL(2); int qemuPhysIfaceConnect(virDomainDefPtr def, @@ -206,7 +206,7 @@ int qemuOpenVhostNet(virDomainDefPtr def, virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, int *vhostfd, - int *vhostfdSize); + size_t *vhostfdSize); int qemuNetworkPrepareDevices(virDomainDefPtr def); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1714341..7cc1c4c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -825,10 +825,10 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; char **tapfdName = NULL; int *tapfd = NULL; - int tapfdSize = 0; + size_t tapfdSize = 0; char **vhostfdName = NULL; int *vhostfd = NULL; - int vhostfdSize = 0; + size_t vhostfdSize = 0; char *nicstr = NULL; char *netstr = NULL; virNetDevVPortProfilePtr vport = NULL; diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 85688ab..83b4131 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -235,7 +235,7 @@ virNetDevProbeVnetHdr(int tapfd) int virNetDevTapCreate(char **ifname, const char *tunpath, int *tapfd, - int tapfdSize, + size_t tapfdSize, unsigned int flags) { size_t i; @@ -370,7 +370,7 @@ int virNetDevTapDelete(const char *ifname, int virNetDevTapCreate(char **ifname, const char *tunpath ATTRIBUTE_UNUSED, int *tapfd, - int tapfdSize, + size_t tapfdSize, unsigned int flags ATTRIBUTE_UNUSED) { int s; @@ -481,7 +481,7 @@ int virNetDevTapDelete(const char *ifname, int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED, const char *tunpath ATTRIBUTE_UNUSED, int *tapfd ATTRIBUTE_UNUSED, - int tapfdSize ATTRIBUTE_UNUSED, + size_t tapfdSize ATTRIBUTE_UNUSED, unsigned int flags ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", @@ -534,7 +534,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, const unsigned char *vmuuid, const char *tunpath, int *tapfd, - int tapfdSize, + size_t tapfdSize, virNetDevVPortProfilePtr virtPortProfile, virNetDevVlanPtr virtVlan, unsigned int flags) diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index c0a4e15..20dec58 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -36,7 +36,7 @@ int virNetDevTapCreate(char **ifname, const char *tunpath, int *tapfd, - int tapfdSize, + size_t tapfdSize, unsigned int flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; @@ -68,7 +68,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, const unsigned char *vmuuid, const char *tunpath, int *tapfd, - int tapfdSize, + size_t tapfdSize, virNetDevVPortProfilePtr virtPortProfile, virNetDevVlanPtr virtVlan, unsigned int flags) -- 2.1.0
On 14.01.2015 15:05, Daniel P. Berrange wrote:
This is just a few patches working towards an eventual goal of being ble to tell system the tap/veth devices associated with a qemu/lxc guest.
Daniel P. Berrange (4): Add support for systemd-machined CreateMachineWithNetwork Add systemd/dtrace probes for DBus APIs Log dtrace/systemd probes at INFO level instead of DEBUG Change int to size_t in size var for tap/vhost FDs
po/POTFILES.in | 1 + src/libvirt_probes.d | 5 ++ src/lxc/lxc_cgroup.c | 1 + src/qemu/qemu_cgroup.c | 1 + src/qemu/qemu_command.c | 18 +++---- src/qemu/qemu_command.h | 10 ++-- src/qemu/qemu_hotplug.c | 4 +- src/util/vircgroup.c | 8 ++++ src/util/vircgroup.h | 2 + src/util/virdbus.c | 26 +++++++++-- src/util/virnetdevtap.c | 8 ++-- src/util/virnetdevtap.h | 4 +- src/util/virprobe.h | 10 ++-- src/util/virsystemd.c | 122 +++++++++++++++++++++++++++++++++++++----------- src/util/virsystemd.h | 2 + tests/virsystemdtest.c | 36 ++++++++++++++ 16 files changed, 202 insertions(+), 56 deletions(-)
ACK series Michal
participants (3)
-
Daniel P. Berrange -
Michal Privoznik -
Zhu Guihua