]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
vmspawn: heap-allocate each DriveInfo individually
authorChristian Brauner <brauner@kernel.org>
Thu, 16 Apr 2026 22:23:20 +0000 (00:23 +0200)
committerChristian Brauner <brauner@kernel.org>
Fri, 24 Apr 2026 12:39:24 +0000 (14:39 +0200)
Change DriveInfos from a contiguous array of DriveInfo structs to an
array of pointers to individually heap-allocated entries. Make all
DriveInfo string fields owned (strdup'd) and add drive_info_new()/
drive_info_free() constructors matching the cleanup pattern.

This prepares for the block device hotplug work where drives need to
be handed off to a hashmap via TAKE_PTR without copying.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
src/vmspawn/vmspawn-qmp.c
src/vmspawn/vmspawn-qmp.h
src/vmspawn/vmspawn.c

index 32af5e4e3e456a724cb00c8614aca76070f91d4e..d467d68a8963612467aff71dfd17be891d6e03f2 100644 (file)
@@ -25,19 +25,37 @@ DEFINE_PRIVATE_HASH_OPS_FULL(
                 char, string_hash_func, string_compare_func, free,
                 PendingJob, pending_job_free);
 
-void drive_info_done(DriveInfo *info) {
-        assert(info);
-        info->serial = mfree(info->serial);
-        info->node_name = mfree(info->node_name);
-        info->pcie_port = mfree(info->pcie_port);
-        info->fd = safe_close(info->fd);
-        info->overlay_fd = safe_close(info->overlay_fd);
+DriveInfo* drive_info_new(void) {
+        DriveInfo *d = new(DriveInfo, 1);
+        if (!d)
+                return NULL;
+
+        *d = (DriveInfo) {
+                .fd = -EBADF,
+                .overlay_fd = -EBADF,
+        };
+        return d;
+}
+
+DriveInfo* drive_info_free(DriveInfo *d) {
+        if (!d)
+                return NULL;
+
+        free(d->path);
+        free(d->format);
+        free(d->disk_driver);
+        free(d->serial);
+        free(d->node_name);
+        free(d->pcie_port);
+        safe_close(d->fd);
+        safe_close(d->overlay_fd);
+        return mfree(d);
 }
 
 void drive_infos_done(DriveInfos *infos) {
         assert(infos);
         FOREACH_ARRAY(d, infos->drives, infos->n_drives)
-                drive_info_done(d);
+                drive_info_free(*d);
         infos->drives = mfree(infos->drives);
         infos->n_drives = 0;
         infos->scsi_pcie_port = mfree(infos->scsi_pcie_port);
@@ -748,7 +766,7 @@ static bool drives_need_scsi_controller(DriveInfos *drives) {
         assert(drives);
 
         FOREACH_ARRAY(d, drives->drives, drives->n_drives)
-                if (STR_IN_SET(d->disk_driver, "scsi-hd", "scsi-cd"))
+                if (STR_IN_SET((*d)->disk_driver, "scsi-hd", "scsi-cd"))
                         return true;
 
         return false;
@@ -792,7 +810,7 @@ int vmspawn_qmp_setup_drives(VmspawnQmpBridge *bridge, DriveInfos *drives) {
         }
 
         FOREACH_ARRAY(d, drives->drives, drives->n_drives) {
-                r = qmp_setup_drive(bridge, qmp, d);
+                r = qmp_setup_drive(bridge, qmp, *d);
                 if (r < 0)
                         return r;
         }
index 15949c40a5e83eb6deecc17e64e28bc82dbaba19..35cc029ea763a8b6fa00dc9ac2690e71f511fc71 100644 (file)
@@ -65,23 +65,27 @@ typedef enum QmpDriveFlags {
         QMP_DRIVE_DISCARD_NO_UNREF = 1u << 6,  /* qcow2 only */
 } QmpDriveFlags;
 
-/* Drive info for QMP-based drive setup */
+/* Drive info for QMP-based drive setup. All string fields are owned.
+ * Each DriveInfo is individually heap-allocated so it can be handed off
+ * to the block device registry via TAKE_PTR. */
 typedef struct DriveInfo {
-        const char *path;          /* kept for logging only — not passed to QEMU */
-        const char *format;        /* "raw" or "qcow2" */
-        const char *disk_driver;   /* "virtio-blk-pci", "scsi-hd", "scsi-cd", "nvme" */
-        char *serial;              /* owned */
-        char *node_name;           /* owned */
-        char *pcie_port;           /* owned: pcie-root-port id for device_add bus (NULL on non-PCIe) */
-        int fd;                    /* pre-opened image fd (owned, -EBADF if unused) */
-        int overlay_fd;            /* pre-opened anonymous overlay fd for ephemeral (owned, -EBADF if unused) */
+        char *path;                /* original path (for logging; not passed to QEMU) */
+        char *format;              /* "raw" or "qcow2" */
+        char *disk_driver;         /* "virtio-blk-pci", "scsi-hd", "scsi-cd", "nvme" */
+        char *serial;
+        char *node_name;
+        char *pcie_port;           /* pcie-root-port id for device_add bus (NULL on non-PCIe) */
+        int fd;                    /* pre-opened image fd (-EBADF if unused) */
+        int overlay_fd;            /* pre-opened anonymous overlay fd for ephemeral (-EBADF if unused) */
         QmpDriveFlags flags;
 } DriveInfo;
 
-void drive_info_done(DriveInfo *info);
+DriveInfo* drive_info_new(void);
+DriveInfo* drive_info_free(DriveInfo *d);
+DEFINE_TRIVIAL_CLEANUP_FUNC(DriveInfo *, drive_info_free);
 
 typedef struct DriveInfos {
-        DriveInfo *drives;
+        DriveInfo **drives;     /* array of individually heap-allocated entries */
         size_t n_drives;
         char *scsi_pcie_port;  /* owned: pcie-root-port id for SCSI controller (NULL if no SCSI or non-PCIe) */
 } DriveInfos;
index e0ddcee9b16b1e313ba14ee6018f21191d1614fb..01325bf9eb1e74033fe4b874fa66cdfa9fc244ed 100644 (file)
@@ -2303,6 +2303,8 @@ static int qemu_config_add_qmp_monitor(FILE *config_file, int bridge_fds[2], int
 }
 
 static int resolve_disk_driver(DiskType dt, const char *filename, DriveInfo *info) {
+        const char *driver;
+        size_t serial_max;
         int r;
 
         assert(filename);
@@ -2310,34 +2312,34 @@ static int resolve_disk_driver(DiskType dt, const char *filename, DriveInfo *inf
 
         switch (dt) {
         case DISK_TYPE_VIRTIO_BLK:
-                info->disk_driver = "virtio-blk-pci";
-                r = disk_serial(filename, DISK_SERIAL_MAX_LEN_VIRTIO_BLK, &info->serial);
-                if (r < 0)
-                        return r;
+                driver = "virtio-blk-pci";
+                serial_max = DISK_SERIAL_MAX_LEN_VIRTIO_BLK;
                 break;
         case DISK_TYPE_VIRTIO_SCSI:
-                info->disk_driver = "scsi-hd";
-                r = disk_serial(filename, DISK_SERIAL_MAX_LEN_SCSI, &info->serial);
-                if (r < 0)
-                        return r;
+                driver = "scsi-hd";
+                serial_max = DISK_SERIAL_MAX_LEN_SCSI;
                 break;
         case DISK_TYPE_NVME:
-                info->disk_driver = "nvme";
-                r = disk_serial(filename, DISK_SERIAL_MAX_LEN_NVME, &info->serial);
-                if (r < 0)
-                        return r;
+                driver = "nvme";
+                serial_max = DISK_SERIAL_MAX_LEN_NVME;
                 break;
         case DISK_TYPE_VIRTIO_SCSI_CDROM:
-                info->disk_driver = "scsi-cd";
+                driver = "scsi-cd";
+                serial_max = DISK_SERIAL_MAX_LEN_SCSI;
                 info->flags |= QMP_DRIVE_READ_ONLY;
-                r = disk_serial(filename, DISK_SERIAL_MAX_LEN_SCSI, &info->serial);
-                if (r < 0)
-                        return r;
                 break;
         default:
                 assert_not_reached();
         }
 
+        info->disk_driver = strdup(driver);
+        if (!info->disk_driver)
+                return log_oom();
+
+        r = disk_serial(filename, serial_max, &info->serial);
+        if (r < 0)
+                return r;
+
         return 0;
 }
 
@@ -2355,8 +2357,9 @@ static int prepare_primary_drive(const char *runtime_dir, DriveInfos *drives) {
         if (r < 0)
                 return log_error_errno(r, "Failed to extract filename from path '%s': %m", arg_image);
 
-        DriveInfo *d = &drives->drives[drives->n_drives++];
-        *d = (DriveInfo) { .fd = -EBADF, .overlay_fd = -EBADF };
+        _cleanup_(drive_info_freep) DriveInfo *d = drive_info_new();
+        if (!d)
+                return log_oom();
 
         r = resolve_disk_driver(arg_image_disk_type, image_fn, d);
         if (r < 0)
@@ -2376,10 +2379,10 @@ static int prepare_primary_drive(const char *runtime_dir, DriveInfos *drives) {
         if (r < 0)
                 return log_error_errno(r, "Expected regular file or block device for image: %s", arg_image);
 
-        d->path = arg_image;
-        d->format = image_format_to_string(arg_image_format);
+        d->path = strdup(arg_image);
+        d->format = strdup(ASSERT_PTR(image_format_to_string(arg_image_format)));
         d->node_name = strdup("vmspawn");
-        if (!d->node_name)
+        if (!d->path || !d->format || !d->node_name)
                 return log_oom();
         d->fd = TAKE_FD(image_fd);
         if (S_ISBLK(st.st_mode))
@@ -2406,6 +2409,7 @@ static int prepare_primary_drive(const char *runtime_dir, DriveInfos *drives) {
                 d->flags |= QMP_DRIVE_NO_FLUSH;
         }
 
+        drives->drives[drives->n_drives++] = TAKE_PTR(d);
         return 0;
 }
 
@@ -2423,8 +2427,9 @@ static int prepare_extra_drives(DriveInfos *drives) {
 
                 DiskType dt = drive->disk_type >= 0 ? drive->disk_type : arg_image_disk_type;
 
-                DriveInfo *d = &drives->drives[drives->n_drives++];
-                *d = (DriveInfo) { .fd = -EBADF, .overlay_fd = -EBADF };
+                _cleanup_(drive_info_freep) DriveInfo *d = drive_info_new();
+                if (!d)
+                        return log_oom();
 
                 r = resolve_disk_driver(dt, drive_fn, d);
                 if (r < 0)
@@ -2445,8 +2450,10 @@ static int prepare_extra_drives(DriveInfos *drives) {
                                                "Block device '%s' cannot be used with 'qcow2' format, only 'raw' is supported.",
                                                drive->path);
 
-                d->path = drive->path;
-                d->format = image_format_to_string(drive->format);
+                d->path = strdup(drive->path);
+                d->format = strdup(ASSERT_PTR(image_format_to_string(drive->format)));
+                if (!d->path || !d->format)
+                        return log_oom();
                 d->fd = TAKE_FD(drive_fd);
                 if (S_ISBLK(drive_st.st_mode))
                         d->flags |= QMP_DRIVE_BLOCK_DEVICE;
@@ -2454,6 +2461,8 @@ static int prepare_extra_drives(DriveInfos *drives) {
 
                 if (asprintf(&d->node_name, "vmspawn_extra_%zu", extra_idx++) < 0)
                         return log_oom();
+
+                drives->drives[drives->n_drives++] = TAKE_PTR(d);
         }
 
         return 0;
@@ -2477,11 +2486,11 @@ static int assign_pcie_ports(MachineConfig *c) {
         /* Drives: non-SCSI drives get individual ports, SCSI controller gets one port */
         bool need_scsi = false;
         FOREACH_ARRAY(d, drives->drives, drives->n_drives) {
-                if (STR_IN_SET(d->disk_driver, "scsi-hd", "scsi-cd")) {
+                if (STR_IN_SET((*d)->disk_driver, "scsi-hd", "scsi-cd")) {
                         need_scsi = true;
                         continue;
                 }
-                if (asprintf(&d->pcie_port, "vmspawn-pcieport-%zu", port++) < 0)
+                if (asprintf(&(*d)->pcie_port, "vmspawn-pcieport-%zu", port++) < 0)
                         return log_oom();
         }
         if (need_scsi)
@@ -2513,7 +2522,7 @@ static int prepare_device_info(const char *runtime_dir, MachineConfig *c) {
 
         /* Build drive info for QMP-based setup. vmspawn opens all image files and
          * passes fds to QEMU via add-fd — QEMU never needs filesystem access. */
-        drives->drives = new0(DriveInfo, 1 + arg_extra_drives.n_drives);
+        drives->drives = new0(DriveInfo*, 1 + arg_extra_drives.n_drives);
         if (!drives->drives)
                 return log_oom();