fix: broken ESM entry, colliding net ids, daemonize pidfile races

package.json

@@ -4,11 +4,11 @@
   "description": "QEMU management library for Node.js",
   "type": "module",
   "main": "./dist/index.cjs",
-  "module": "./dist/index.es",
+  "module": "./dist/index.mjs",
   "types": "./dist/index.d.ts",
   "exports": {
     ".": {
-      "import": "./dist/index.es",
+      "import": "./dist/index.mjs",
       "require": "./dist/index.cjs",
       "types": "./dist/index.d.ts"
     }

src/process/args.ts

@@ -51,7 +51,15 @@ export function buildArgs(config: QemuConfig): string[] {
 
   (config.drives ?? []).forEach((drive, i) => {
     const id = drive.id ?? (drive.virtio ? `drive${i}` : undefined);
-    const parts: string[] = [`file=${drive.file}`, `format=${drive.format ?? "raw"}`];
+    const parts: string[] = [];
+    // A virtio drive is attached to its virtio-blk-pci device by the
+    // separate -device clause below. Without if=none, QEMU's legacy
+    // "-drive without if= defaults to if=ide" behavior auto-attaches it
+    // to an IDE device too, and the drive ends up attached twice: QEMU
+    // rejects the second (virtio-blk-pci) attach with "Drive 'driveN' is
+    // already in use ... did you need 'if=none'?".
+    if (drive.virtio) parts.push("if=none");
+    parts.push(`file=${drive.file}`, `format=${drive.format ?? "raw"}`);
     if (id) parts.push(`id=${id}`);
     if (drive.media) parts.push(`media=${drive.media}`);
     if (drive.cache) parts.push(`cache=${drive.cache}`);
@@ -62,9 +70,7 @@ export function buildArgs(config: QemuConfig): string[] {
     if (drive.virtio) args.push("-device", `virtio-blk-pci,drive=${id}`);
   });
 
-  for (const net of config.net ?? []) {
-    appendNet(args, net);
-  }
+  (config.net ?? []).forEach((net, i) => appendNet(args, net, i));
 
   if (config.vnc) {
     let v = config.vnc.display;
@@ -90,13 +96,17 @@ export function buildArgs(config: QemuConfig): string[] {
   return args;
 }
 
-function appendNet(args: string[], net: QemuNet): void {
+function appendNet(args: string[], net: QemuNet, index: number): void {
   if (net.type === "none") {
     args.push("-nic", "none");
     return;
   }
 
-  const id = net.id ?? "net0";
+  // Every net entry defaulted to the literal "net0" regardless of
+  // position, unlike drives' per-index default id -- multiple net
+  // entries without an explicit id collided with "Duplicate ID 'net0'
+  // for netdev".
+  const id = net.id ?? `net${index}`;
 
   if (net.type === "tap") {
     const parts = [`tap`, `id=${id}`];

src/process/manager.ts

@@ -71,7 +71,13 @@ export class QemuProcess extends EventEmitter<QemuProcessEventMap> {
 
     this.proc = proc;
 
-    if (this.config.pidfile && proc.pid != null) {
+    // With daemonize:true, QEMU itself forks and writes config.pidfile
+    // (via the -pidfile arg in buildArgs) with the *real* detached
+    // daemon's pid once it's ready — this spawned wrapper process is
+    // just the launcher and its own pid is not the one that ends up
+    // running. Writing it here would race qemu's own write with the
+    // wrong pid, so skip it in that case.
+    if (this.config.pidfile && proc.pid != null && !this.config.daemonize) {
       await writeFile(this.config.pidfile, String(proc.pid), "utf8");
     }
 
@@ -84,7 +90,12 @@ export class QemuProcess extends EventEmitter<QemuProcessEventMap> {
 
     proc.on("exit", (code, signal) => {
       this.proc = null;
-      if (this.config.pidfile) {
+      // With daemonize:true, this "exit" is the launcher forking and
+      // returning — expected, successful, and unrelated to whether the
+      // real daemon (still running, detached) is alive. Deleting the
+      // pidfile here would delete the live daemon's own pidfile out
+      // from under it.
+      if (this.config.pidfile && !this.config.daemonize) {
         rm(this.config.pidfile, { force: true }).catch(() => {});
       }
       this.emit("exit", code, signal);

tests/process/args.test.ts

@@ -118,6 +118,46 @@ describe("buildArgs", () => {
     expect(args[idx + 1]).toContain("discard=unmap");
   });
 
+  it("adds if=none for a virtio drive, so the -device attach doesn't fight QEMU's implicit if=ide attach", () => {
+    const args = buildArgs({
+      drives: [{ file: "/vm.qcow2", format: "qcow2", virtio: true }],
+    });
+    const idx = args.indexOf("-drive");
+    expect(args[idx + 1]).toContain("if=none");
+    expect(args).toContain("-device");
+    expect(args[args.indexOf("-device") + 1]).toBe("virtio-blk-pci,drive=drive0");
+  });
+
+  it("does not add if=none for a non-virtio drive", () => {
+    const args = buildArgs({
+      drives: [{ file: "/seed.img", format: "raw", media: "cdrom" }],
+    });
+    const idx = args.indexOf("-drive");
+    expect(args[idx + 1]).not.toContain("if=none");
+  });
+
+  it("gives each net entry a distinct default id instead of always 'net0'", () => {
+    const args = buildArgs({
+      net: [
+        { type: "tap", ifname: "tap0", script: "no", downscript: "no" },
+        { type: "tap", ifname: "tap1", script: "no", downscript: "no" },
+        { type: "tap", ifname: "tap2", script: "no", downscript: "no" },
+      ],
+    });
+    const netdevs = args.filter((a) => a.startsWith("tap,id="));
+    expect(netdevs).toEqual([
+      "tap,id=net0,ifname=tap0,script=no,downscript=no",
+      "tap,id=net1,ifname=tap1,script=no,downscript=no",
+      "tap,id=net2,ifname=tap2,script=no,downscript=no",
+    ]);
+    const devices = args.filter((_, i) => args[i - 1] === "-device");
+    expect(devices).toEqual([
+      "virtio-net-pci,netdev=net0",
+      "virtio-net-pci,netdev=net1",
+      "virtio-net-pci,netdev=net2",
+    ]);
+  });
+
   it("adds tap network", () => {
     const args = buildArgs({
       net: [{ type: "tap", id: "net0", ifname: "tap0", script: "no", downscript: "no" }],

tests/process/manager.test.ts

@@ -5,6 +5,7 @@ vi.mock("node:child_process", () => ({ spawn: vi.fn() }));
 vi.mock("node:fs/promises", () => ({ writeFile: vi.fn().mockResolvedValue(undefined), rm: vi.fn().mockResolvedValue(undefined) }));
 
 import { spawn } from "node:child_process";
+import { writeFile, rm } from "node:fs/promises";
 import { QemuProcess } from "../../src/process/manager.js";
 
 class MockProcess extends EventEmitter {
@@ -88,6 +89,35 @@ describe("QemuProcess", () => {
     expect(spy).toHaveBeenCalledWith(0, null);
   });
 
+  it("writes the pidfile itself when not daemonized", async () => {
+    const mock = new MockProcess();
+    vi.mocked(spawn).mockReturnValue(mock as unknown as ReturnType<typeof spawn>);
+    const proc = new QemuProcess({ config: { pidfile: "/run/vm.pid" } });
+    await proc.start();
+    expect(writeFile).toHaveBeenCalledWith("/run/vm.pid", "12345", "utf8");
+  });
+
+  it("deletes the pidfile on exit when not daemonized", async () => {
+    const mock = new MockProcess();
+    vi.mocked(spawn).mockReturnValue(mock as unknown as ReturnType<typeof spawn>);
+    const proc = new QemuProcess({ config: { pidfile: "/run/vm.pid" } });
+    await proc.start();
+    mock.emit("exit", 0, null);
+    expect(rm).toHaveBeenCalledWith("/run/vm.pid", { force: true });
+  });
+
+  it("does not touch the pidfile when daemonized, since QEMU itself owns it after forking", async () => {
+    const mock = new MockProcess();
+    vi.mocked(spawn).mockReturnValue(mock as unknown as ReturnType<typeof spawn>);
+    const proc = new QemuProcess({ config: { pidfile: "/run/vm.pid", daemonize: true } });
+    await proc.start();
+    expect(writeFile).not.toHaveBeenCalled();
+    // The wrapper process exiting (code 0) is normal daemonize behavior,
+    // not the real daemon dying -- must not delete the daemon's pidfile.
+    mock.emit("exit", 0, null);
+    expect(rm).not.toHaveBeenCalled();
+  });
+
   it("implements Symbol.asyncDispose", () => {
     expect(typeof makeProc()[Symbol.asyncDispose]).toBe("function");
   });

vite.config.ts

@@ -7,7 +7,7 @@ export default defineConfig({
     lib: {
       entry: "src/index.ts",
       formats: ["es", "cjs"],
-      fileName: (format) => format === "es" ? "index.es" : "index.cjs",
+      fileName: (format) => format === "es" ? "index.mjs" : "index.cjs",
     },
     rollupOptions: {
       external: [/^node:/],