[PATCH 0/3] qemu: fix validation of 'fd' passing for backup job and improve debugability of passed FDs
10:18 a.m.
Peter Krempa (3):
qemuBackupPrepare: Actually allow 'VIR_STORAGE_NET_HOST_TRANS_FD'
docs: backup: Hint at proper selinux labelling of the FD-passed NBD
socket
qemu: fd: Log information about passed file descriptor
docs/formatbackup.rst | 4 +++
src/qemu/qemu_backup.c | 4 ++-
src/qemu/qemu_fd.c | 62 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 69 insertions(+), 1 deletion(-)
--
2.49.0
10:18 a.m.
New subject: [PATCH 1/3] qemuBackupPrepare: Actually allow 'VIR_STORAGE_NET_HOST_TRANS_FD'
From: Peter Krempa <pkrempa(a)redhat.com>
While I've actually implemented support for FD passing the NBD server
socket in eb768a556db I managed to misplace the hunk allowing the 'FD'
transport in the validation code, rendering the whole feature useless.
Fix the validation logic to make the feature useful.
Fixes: eb768a556db75040f7b518d198a18bd0f5d6faad
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_backup.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index f6ee31dc2a..fb3558d280 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -86,8 +86,10 @@ qemuBackupPrepare(virDomainBackupDef *def)
/* TODO: Do we need to mess with selinux? */
break;
- case VIR_STORAGE_NET_HOST_TRANS_RDMA:
case VIR_STORAGE_NET_HOST_TRANS_FD:
+ break;
+
+ case VIR_STORAGE_NET_HOST_TRANS_RDMA:
case VIR_STORAGE_NET_HOST_TRANS_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("unexpected transport in <domainbackup>"));
--
2.49.0
11:37 a.m.
New subject: [PATCH 1/3] qemuBackupPrepare: Actually allow
'VIR_STORAGE_NET_HOST_TRANS_FD'
On a Monday in 2025, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa(a)redhat.com>
While I've actually implemented support for FD passing the NBD server
socket in eb768a556db I managed to misplace the hunk allowing the 'FD'
transport in the validation code, rendering the whole feature useless.
Fix the validation logic to make the feature useful.
s/useful/usable/?
Fixes: eb768a556db75040f7b518d198a18bd0f5d6faad
The commit that misplaced the case is:
commit ee49106dbf6f3c716f0e31c9ecc9bf528eb72bb2
conf: Introduce VIR_STORAGE_NET_HOST_TRANS_FD
Not the one you mention (twice).
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_backup.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1:19 p.m.
New subject: [PATCH 1/3] qemuBackupPrepare: Actually allow
'VIR_STORAGE_NET_HOST_TRANS_FD'
On Mon, May 19, 2025 at 18:37:03 +0200, Ján Tomko wrote:
On a Monday in 2025, Peter Krempa via Devel wrote:
> From: Peter Krempa <pkrempa(a)redhat.com>
>
> While I've actually implemented support for FD passing the NBD server
> socket in eb768a556db I managed to misplace the hunk allowing the 'FD'
> transport in the validation code, rendering the whole feature useless.
>
> Fix the validation logic to make the feature useful.
s/useful/usable/?
>
> Fixes: eb768a556db75040f7b518d198a18bd0f5d6faad
The commit that misplaced the case is:
commit ee49106dbf6f3c716f0e31c9ecc9bf528eb72bb2
conf: Introduce VIR_STORAGE_NET_HOST_TRANS_FD
Not the one you mention (twice).
That commit placed it correctly (at that point in time) because the
implementation wasn't there. The commit I'm mentioning was supposed to
allow it (I mentioned "misplacing hunk allowing the 'FD' transport').
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_backup.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
10:18 a.m.
New subject: [PATCH 2/3] docs: backup: Hint at proper selinux labelling of the FD-passed NBD socket
From: Peter Krempa <pkrempa(a)redhat.com>
In case selinux is used on the host the socket passed to qemu needs to
be properly labelled. Add a hint to the example code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/formatbackup.rst | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/docs/formatbackup.rst b/docs/formatbackup.rst
index 155a45a22f..df6392e3bd 100644
--- a/docs/formatbackup.rst
+++ b/docs/formatbackup.rst
@@ -53,6 +53,10 @@ were supplied). The following child elements and attributes are
supported:
import socket
import libvirt
+ import selinux
+
+ # Optionally setup selinux context for the socket if the distro uses it
+ # selinux.setsockcreatecon_raw("system_u:object_r:svirt_t:s0")
s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
s.bind("/path/to/socket")
--
2.49.0
11:41 a.m.
New subject: [PATCH 2/3] docs: backup: Hint at proper selinux labelling of
the FD-passed NBD socket
On a Monday in 2025, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa(a)redhat.com>
In case selinux is used on the host the socket passed to qemu needs to
be properly labelled. Add a hint to the example code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/formatbackup.rst | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/docs/formatbackup.rst b/docs/formatbackup.rst
index 155a45a22f..df6392e3bd 100644
--- a/docs/formatbackup.rst
+++ b/docs/formatbackup.rst
@@ -53,6 +53,10 @@ were supplied). The following child elements and attributes are
supported:
import socket
import libvirt
+ import selinux
+
+ # Optionally setup selinux context for the socket if the distro uses it
+ # selinux.setsockcreatecon_raw("system_u:object_r:svirt_t:s0")
Should this hint that the category should also match the domain, to be
"properly" labelled?
Either way:
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
s.bind("/path/to/socket")
--
2.49.0
1:20 p.m.
New subject: [PATCH 2/3] docs: backup: Hint at proper selinux labelling of
the FD-passed NBD socket
On Mon, May 19, 2025 at 18:41:45 +0200, Ján Tomko wrote:
On a Monday in 2025, Peter Krempa via Devel wrote:
> From: Peter Krempa <pkrempa(a)redhat.com>
>
> In case selinux is used on the host the socket passed to qemu needs to
> be properly labelled. Add a hint to the example code.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> docs/formatbackup.rst | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/docs/formatbackup.rst b/docs/formatbackup.rst
> index 155a45a22f..df6392e3bd 100644
> --- a/docs/formatbackup.rst
> +++ b/docs/formatbackup.rst
> @@ -53,6 +53,10 @@ were supplied). The following child elements and attributes are
supported:
>
> import socket
> import libvirt
> + import selinux
> +
> + # Optionally setup selinux context for the socket if the distro uses it
> + # selinux.setsockcreatecon_raw("system_u:object_r:svirt_t:s0")
Should this hint that the category should also match the domain, to be
"properly" labelled?
Well, possibly; but this is actually enough to make it work. I didn't
care digging deeper.
10:18 a.m.
New subject: [PATCH 3/3] qemu: fd: Log information about passed file descriptor
From: Peter Krempa <pkrempa(a)redhat.com>
Log information (type, label, etc) about FDs passed to qemu via APIs
from this module.
This does "spill" the selinux library code into this module, but
acessing it via the security driver would require passing much more
context to this module. Since it's for logging only it can be easily
removed if necessary.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_fd.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/src/qemu/qemu_fd.c b/src/qemu/qemu_fd.c
index 333f9b128e..ef0a8d8127 100644
--- a/src/qemu/qemu_fd.c
+++ b/src/qemu/qemu_fd.c
@@ -25,6 +25,11 @@
#include "virfile.h"
#include "virlog.h"
+/* Used strictly for logging selinux context of passed FD */
+#ifdef WITH_SECDRIVER_SELINUX
+# include <selinux/selinux.h>
+#endif
+
#define VIR_FROM_THIS VIR_FROM_QEMU
VIR_LOG_INIT("qemu.qemu_fd");
@@ -44,6 +49,56 @@ struct _qemuFDPass {
};
+static void
+qemuFDPassLogFDInfo(const char *name,
+ size_t idx,
+ int fd)
+{
+ struct stat st;
+ const char *type = "error";
+ g_autofree char *selinux = NULL;
+ g_autofree char *tmp = NULL;
+
+ if (fstat(fd, &st) == 0) {
+ switch (st.st_mode & S_IFMT) {
+ case S_IFBLK:
+ type = "block";
+ break;
+ case S_IFCHR:
+ type = "char";
+ break;
+ case S_IFDIR:
+ type = "directory";
+ break;
+ case S_IFIFO:
+ type = "pipe";
+ break;
+ case S_IFLNK:
+ type = "symlink";
+ break;
+ case S_IFREG:
+ type = "file";
+ break;
+ case S_IFSOCK:
+ type = "socket";
+ break;
+ default:
+ type = tmp = g_strdup_printf("unknown:'0x%x')",
st.st_mode & S_IFMT);
+ break;
+ }
+ }
+
+#ifdef WITH_SECDRIVER_SELINUX
+ ignore_value(fgetfilecon_raw(fd, &selinux));
+#else
+ selinux = g_strdup("N/A");
+#endif
+
+ VIR_DEBUG("passing fd:'%i', name:'%s'(%zu) type:'%s'
selinux:'%s'",
+ fd, name, idx, type, selinux);
+}
+
+
void
qemuFDPassFree(qemuFDPass *fdpass)
{
@@ -234,6 +289,8 @@ qemuFDPassTransferCommand(qemuFDPass *fdpass,
fdpass->fds[i].fd,
fdpass->fds[i].opaque);
+ qemuFDPassLogFDInfo(fdpass->fds[i].opaque, i, fdpass->fds[i].fd);
+
virCommandPassFD(cmd, fdpass->fds[i].fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
fdpass->fds[i].fd = -1;
virCommandAddArgList(cmd, "-add-fd", arg, NULL);
@@ -274,6 +331,8 @@ qemuFDPassTransferMonitor(qemuFDPass *fdpass,
}
for (i = 0; i < fdpass->nfds; i++) {
+ qemuFDPassLogFDInfo(fdpass->fds[i].opaque, i, fdpass->fds[i].fd);
+
if (qemuMonitorAddFileHandleToSet(mon,
fdpass->fds[i].fd,
fdpass->fdSetID,
@@ -381,6 +440,7 @@ qemuFDPassDirectTransferCommand(qemuFDPassDirect *fdpass,
if (!fdpass)
return;
+ qemuFDPassLogFDInfo(fdpass->name, 0, fdpass->fd);
virCommandPassFD(cmd, fdpass->fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
g_free(fdpass->name);
fdpass->name = g_strdup_printf("%d", fdpass->fd);
@@ -403,6 +463,8 @@ qemuFDPassDirectTransferMonitor(qemuFDPassDirect *fdpass,
if (!fdpass)
return 0;
+ qemuFDPassLogFDInfo(fdpass->name, 0, fdpass->fd);
+
if (qemuMonitorSendFileHandle(mon, fdpass->name, fdpass->fd) < 0)
return -1;
--
2.49.0
11:42 a.m.
New subject: [PATCH 3/3] qemu: fd: Log information about passed file
descriptor
On a Monday in 2025, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa(a)redhat.com>
Log information (type, label, etc) about FDs passed to qemu via APIs
from this module.
This does "spill" the selinux library code into this module, but
acessing it via the security driver would require passing much more
context to this module. Since it's for logging only it can be easily
removed if necessary.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_fd.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
50
days inactive
50
days old
8 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa