Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libxl/blktap integration #1491

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions recipes-core/packagegroups/packagegroup-xenclient-dom0.bb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ RDEPENDS_${PN} = " \
tpm2-tss \
tpm2-tools \
blktap3 \
tapback \
pesign \
ipxe \
udev-extraconf-dom0 \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,3 @@ PATCHES:
block-obj-$(CONFIG_PARALLELS) += parallels.o
block-obj-y += blklogwrites.o
block-obj-y += block-backend.o snapshot.o qapi.o
@@ -24,6 +29,6 @@ block-obj-y += throttle-groups.o
block-obj-$(CONFIG_LINUX) += nvme.o

-block-obj-y += nbd.o
+block-obj-n += nbd.o
block-obj-$(CONFIG_SHEEPDOG) += sheepdog.o
block-obj-$(CONFIG_LIBISCSI) += iscsi.o
block-obj-$(if $(CONFIG_LIBISCSI),y,n) += iscsi-opts.o
3 changes: 3 additions & 0 deletions recipes-extended/xen/blktap3.bb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ SRC_URI = "git://github.com/xapi-project/blktap.git;protocol=https \
file://gcc9-compilation.patch \
file://openssl-1.1.x.patch \
file://0001-tap-ctl-Default-to-read-only-opening.patch \
file://0001-tapback-Add-l-libxl-compatibility-mode.patch \
file://0002-tapback-Move-backend-to-InitWait-earlier.patch \
file://0003-tapback-Don-t-remove-xenstore-backend-in-libxl-mode.patch \
"

S = "${WORKDIR}/git"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
From 63f3883abe1f94a85428e17e98fe0dbe15606b0b Mon Sep 17 00:00:00 2001
From: Jason Andryuk <[email protected]>
Date: Thu, 25 Jan 2024 08:22:55 -0500
Subject: [PATCH 1/3] tapback: Add -l/--libxl compatibility mode

libxl and xapi/xenopsd have different ordering requirements, and the
libxl required changes are not readily compatible with xapi.

Add a flag to allow running in libxl compatibility mode to bodge around
the issue. The flag has to be passed from the parent tapback process
into the per-domain ones to ensure it is respected.

Some nearby hard tabs are converted to spaces.

Signed-off-by: Jason Andryuk <[email protected]>
---
tapback/backend.c | 8 +++++---
tapback/tapback.c | 17 ++++++++++++++---
tapback/tapback.h | 10 ++++++++++
3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/tapback/backend.c b/tapback/backend.c
index facae9b..cb607b1 100644
--- a/tapback/backend.c
+++ b/tapback/backend.c
@@ -1197,7 +1197,7 @@ tapback_backend_handle_backend_watch(backend_t *backend,
* FIXME Shall we watch the child process?
*/
} else { /* child */
- char *args[7];
+ char *args[8];
int i = 0;

args[i++] = (char*)tapback_name;
@@ -1211,8 +1211,10 @@ tapback_backend_handle_backend_watch(backend_t *backend,
}
if (log_level == LOG_DEBUG)
args[i++] = "-v";
- if (!backend->barrier)
- args[i++] = "-b";
+ if (!backend->barrier)
+ args[i++] = "-b";
+ if (libxl_mode())
+ args[i++] = "--libxl";
args[i] = NULL;
/*
* TODO we're hard-coding the name of the binary, better let
diff --git a/tapback/tapback.c b/tapback/tapback.c
index fe04d17..6a3098f 100644
--- a/tapback/tapback.c
+++ b/tapback/tapback.c
@@ -58,6 +58,7 @@
const char tapback_name[] = "tapback";
unsigned log_level;
int tapdev_major;
+static bool opt_libxl_mode;

struct list_head backends = LIST_HEAD_INIT(backends);

@@ -77,6 +78,12 @@ char *xenbus_strstate(const XenbusState xbs)
return str[xbs];
}

+bool
+libxl_mode(void)
+{
+ return opt_libxl_mode;
+}
+
/**
* Read changes that occurred on the "backend/<backend name>" XenStore path
* or one of the front-end paths and act accordingly.
@@ -513,7 +520,8 @@ usage(FILE * const stream, const char * const prog)
"\t[-h|--help]\n"
"\t[-v|--verbose]\n"
"\t[-b]--nobarrier]\n"
- "\t[-n|--name]\n", prog);
+ "\t[-n|--name]\n"
+ "\t[-l|--libxl]\n", prog);
}

extern char *optarg;
@@ -612,8 +620,8 @@ int main(int argc, char **argv)
{"name", 0, NULL, 'n'},
{"pidfile", 0, NULL, 'p'},
{"domain", 0, NULL, 'x'},
- {"nobarrier", 0, NULL, 'b'},
-
+ {"nobarrier", 0, NULL, 'b'},
+ {"libxl", 0, NULL, 'l'},
};
int c;

@@ -628,6 +636,9 @@ int main(int argc, char **argv)
case 'd':
opt_debug = true;
break;
+ case 'l':
+ opt_libxl_mode = true;
+ break;
case 'v':
opt_verbose = true;
break;
diff --git a/tapback/tapback.h b/tapback/tapback.h
index 9bde55d..f106f25 100644
--- a/tapback/tapback.h
+++ b/tapback/tapback.h
@@ -524,4 +524,14 @@ tapback_backend_destroy(backend_t *backend);

bool verbose(void);

+/**
+ * Indicates when tapback is running in libxl compatibility mode. libxl needs
+ * backends to transition to InitWait earlier than xapi/xenopds.
+ *
+ * Returns true if tapback is running in libxl compatibility mode and false
+ * otherwise.
+ */
+bool
+libxl_mode(void);
+
#endif /* __TAPBACK_H__ */
--
2.43.0

Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
From 726cdd1e45ed155af48b9b433492a0e427256b65 Mon Sep 17 00:00:00 2001
From: Jason Andryuk <[email protected]>
Date: Fri, 8 Dec 2023 13:17:20 -0500
Subject: [PATCH 2/3] tapback: Move backend to InitWait earlier

This is needed for libxl integration.

tapdisk waits to transition to InitWait until after "hotplug-status" is
connected. However libxl doesn't run its hotplug scripts, which write
"hotplug-status", until the backend transitions to InitWait. With both
sides waiting, progress is not made and connecting the blktap device
times out and fails.

Make tapback transition to InitWait earlier to resolve this issue under
libxl. Place the transition to InitWait in
tapback_backend_create_device() after the xenstore feature nodes have
been written. InitWait is a signal to the frontend that such nodes have
been written. This matches blkback's behaviour. It should also be fine
since tapback still won't advance to Connected without the other setup
like physical-device-path and hotplug-status.

VBDs can be reconnected. When pv-grub is used, it connects the VBD,
loads the kernel, disconnects the VBD. It then re-sets the frontend
state to XenbusStateInitialising so that the new kernel will find and
connect the VBD.

tapback and blkback handle this case differently. When blkback observes
the frontend transition to XenbusStateInitialising, and the backend is
XenbusStateClosed, the backend transitions to XenbusStateInitWait.

When tapback observes the frontend transition to
XenbusStateInitialising, the backend checks for hotplug_status_connected
to be true before switching XenbusStateInitWait. For tapback, this
serves a second purpose for setting XenbusStateInitWait initially as
well.

Use libxl_mode() to determine whether to transition to InitWait sooner.
Leave the hotplug_status_connected check since that should work fine for
either even through it may be an extra watch firing for libxl.

Signed-off-by: Jason Andryuk <[email protected]>
---
tapback/backend.c | 11 +++++++++++
tapback/frontend.c | 4 ++--
2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/tapback/backend.c b/tapback/backend.c
index cb607b1..0494f15 100644
--- a/tapback/backend.c
+++ b/tapback/backend.c
@@ -259,6 +259,17 @@ tapback_backend_create_device(backend_t *backend,
goto out;
}

+ if (libxl_mode()) {
+ /* In libxl_mode, after tapback has written it's capabilities to
+ * XenStore, switch to InitWait. */
+ err = xenbus_switch_state(device, XenbusStateInitWait);
+ if (unlikely(err)) {
+ WARN(device, "failed to switch to XenbusStateInitWait: %s\n",
+ strerror(-err));
+ goto out;
+ }
+ }
+
out:
if (err) {
WARN(NULL, "%s: error creating device: %s\n", name, strerror(-err));
diff --git a/tapback/frontend.c b/tapback/frontend.c
index e086813..1b68d88 100644
--- a/tapback/frontend.c
+++ b/tapback/frontend.c
@@ -439,8 +439,8 @@ frontend_changed(vbd_t * const device, const XenbusState state)

switch (state) {
case XenbusStateInitialising:
- if (device->hotplug_status_connected)
- err = xenbus_switch_state(device, XenbusStateInitWait);
+ if (device->hotplug_status_connected)
+ err = xenbus_switch_state(device, XenbusStateInitWait);
break;
case XenbusStateInitialised:
case XenbusStateConnected:
--
2.43.0

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
From adaffa0a7dab4984570e72c49366a08474bc6f2e Mon Sep 17 00:00:00 2001
From: Jason Andryuk <[email protected]>
Date: Thu, 25 Jan 2024 08:31:57 -0500
Subject: [PATCH 3/3] tapback: Don't remove xenstore backend in libxl mode

libxl doesn't clean up tapdisks because it doesn't call the hotplug
cleanup scripts:
libxl: debug: libxl_event.c:1043:devstate_callback: backend /local/domain/0/backend/vbd3/5/2048/state wanted state 6 but it was removed
libxl: debug: libxl_event.c:849:libxl__ev_xswatch_deregister: watch w=0xf82ba0 wpath=/local/domain/0/backend/vbd3/5/2048/state token=1/2: deregister slotnum=1
libxl: debug: libxl_device.c:1156:device_backend_callback: Domain 5:calling device_backend_cleanup
libxl: debug: libxl_event.c:863:libxl__ev_xswatch_deregister: watch w=0xf82ba0: deregister unregistered
libxl: error: libxl_device.c:1169:device_backend_callback: Domain 5:unable to remove device with path /local/domain/0/backend/vbd3/5/2048 - rc -6

The backend state cannot be found because tapback deleted the entire
backend subtree.

In libxl mode, tapback shouldn't remove the backend nodes when the
frontend is removed, because the nodes contain the information on how to
clean up. Leave the nodes and allowed them to be removed by the
toolstack.

Signed-off-by: Jason Andryuk <[email protected]>
---
tapback/frontend.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tapback/frontend.c b/tapback/frontend.c
index 1b68d88..d5db62b 100644
--- a/tapback/frontend.c
+++ b/tapback/frontend.c
@@ -508,7 +508,7 @@ tapback_backend_handle_otherend_watch(backend_t *backend,
*/
s = tapback_xs_read(device->backend->xs, XBT_NULL, "%s",
device->frontend_state_path);
- if (!s) {
+ if (!s && !libxl_mode()) {
err = errno;
/*
* If the front-end XenBus node is missing, the XenBus device has been
@@ -531,6 +531,9 @@ tapback_backend_handle_otherend_watch(backend_t *backend,
}
}
}
+ } else if (!s && libxl_mode()) {
+ WARN(device, "frontend disappeared!");
+ err = ENOENT;
} else {
state = strtol(s, &end, 0);
if (*end != 0 || end == s) {
--
2.43.0

Original file line number Diff line number Diff line change
Expand Up @@ -83,81 +83,6 @@ PATCHES
if (ret <= 0) {
err = errno;
break;
--- a/include/blktap.h
+++ b/include/blktap.h
@@ -32,7 +32,7 @@
#define _TD_BLKTAP_H_

#define BLKTAP2_SYSFS_DIR "/sys/class/blktap2"
-#define BLKTAP2_CONTROL_NAME "blktap/control"
+#define BLKTAP2_CONTROL_NAME "blktap-control"
#define BLKTAP2_CONTROL_DIR "/var/run/blktap-control"
#define BLKTAP2_CONTROL_SOCKET "ctl"
#define BLKTAP2_DIRECTORY "/dev/xen/blktap-2"
--- a/include/blktap2.h
+++ b/include/blktap2.h
@@ -50,7 +50,7 @@
#define BLKTAP2_IOCTL_REMOVE_DEVICE 207

#define BLKTAP2_SYSFS_DIR "/sys/class/blktap2"
-#define BLKTAP2_CONTROL_NAME "blktap/control"
+#define BLKTAP2_CONTROL_NAME "blktap-control"
#define BLKTAP2_CONTROL_DIR "/var/run/blktap-control"
#define BLKTAP2_CONTROL_SOCKET "ctl"
#define BLKTAP2_DIRECTORY "/dev/xen/blktap-2"
--- a/tapback/frontend.c
+++ b/tapback/frontend.c
@@ -477,7 +477,7 @@ tapback_backend_handle_otherend_watch(ba
{
vbd_t *device = NULL;
int err = 0, state = 0;
- char *s = NULL, *end = NULL, *_path = NULL;
+ char *s = NULL, *end = NULL;

ASSERT(backend);
ASSERT(path);
@@ -508,30 +508,7 @@ tapback_backend_handle_otherend_watch(ba
*/
s = tapback_xs_read(device->backend->xs, XBT_NULL, "%s",
device->frontend_state_path);
- if (!s) {
- err = errno;
- /*
- * If the front-end XenBus node is missing, the XenBus device has been
- * removed: remove the XenBus back-end node.
- */
- if (err == ENOENT) {
- err = asprintf(&_path, "%s/%s/%d/%d", XENSTORE_BACKEND,
- device->backend->name, device->domid, device->devid);
- if (err == -1) {
- err = errno;
- WARN(device, "failed to asprintf: %s\n", strerror(err));
- goto out;
- }
- err = 0;
- if (!xs_rm(device->backend->xs, XBT_NULL, _path)) {
- if (errno != ENOENT) {
- err = errno;
- WARN(device, "failed to remove %s: %s\n", path,
- strerror(err));
- }
- }
- }
- } else {
+ if(s) {
state = strtol(s, &end, 0);
if (*end != 0 || end == s) {
WARN(device, "invalid XenBus state '%s'\n", s);
@@ -540,9 +517,7 @@ tapback_backend_handle_otherend_watch(ba
err = frontend_changed(device, state);
}

-out:
free(s);
- free(_path);
return err;
}

--- a/tapback/tapback.c
+++ b/tapback/tapback.c
@@ -171,6 +171,7 @@ tapback_backend_destroy(backend_t *backe
Expand Down
2 changes: 1 addition & 1 deletion recipes-extended/xen/blktap3/tapback.initscript
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
DESC="Tapback"
EXEC=/usr/bin/tapback
PIDFILE="/var/run/tapback.pid"
OPTS="-p $PIDFILE"
OPTS="-p $PIDFILE --libxl"

do_start() {
start-stop-daemon --start --quiet --oknodo --pidfile "$PIDFILE" \
Expand Down
Loading