Introduce virt-xml to disk update API

cockpit-project · Sep 7, 2022 · 3e6c3f4 · 3e6c3f4
1 parent c52148e
commit 3e6c3f4
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 59 deletions.
diff --git a/src/components/vm/disks/diskEdit.jsx b/src/components/vm/disks/diskEdit.jsx
@@ -204,10 +204,12 @@ export class EditDiskModal extends React.Component {
 
         domainUpdateDiskAttributes({
             connectionName: vm.connectionName,
-            objPath: vm.id, target: disk.target,
+            vmName: vm.name,
+            target: disk.target,
             readonly: this.state.access == "readonly",
             shareable: this.state.access == "shareable",
             busType: this.state.busType,
+            oldBusType: disk.bus,
             cache: this.state.cacheMode,
             existingTargets
         })

diff --git a/src/libvirt-xml-update.js b/src/libvirt-xml-update.js
@@ -1,55 +1,4 @@
 import { getDoc, getSingleOptionalElem } from './libvirt-xml-parse.js';
-import { getNextAvailableTarget } from './helpers.js';
-
-export function updateDisk({ domXml, diskTarget, readonly, shareable, busType, existingTargets, cache }) {
-    const s = new XMLSerializer();
-    const doc = getDoc(domXml);
-    const domainElem = doc.firstElementChild;
-    if (!domainElem)
-        throw new Error("updateBootOrder: domXML has no domain element");
-
-    const deviceElem = domainElem.getElementsByTagName("devices")[0];
-    const disks = deviceElem.getElementsByTagName("disk");
-
-    for (let i = 0; i < disks.length; i++) {
-        const disk = disks[i];
-        const target = disk.getElementsByTagName("target")[0].getAttribute("dev");
-        if (target == diskTarget) {
-            let shareAbleElem = getSingleOptionalElem(disk, "shareable");
-            if (!shareAbleElem && shareable) {
-                shareAbleElem = doc.createElement("shareable");
-                disk.appendChild(shareAbleElem);
-            } else if (shareAbleElem && !shareable) {
-                shareAbleElem.remove();
-            }
-
-            let readOnlyElem = getSingleOptionalElem(disk, "readonly");
-            if (!readOnlyElem && readonly) {
-                readOnlyElem = doc.createElement("readonly");
-                disk.appendChild(readOnlyElem);
-            } else if (readOnlyElem && !readonly) {
-                readOnlyElem.remove();
-            }
-
-            const targetElem = disk.getElementsByTagName("target")[0];
-            const oldBusType = targetElem.getAttribute("bus");
-            if (busType && oldBusType !== busType) {
-                targetElem.setAttribute("bus", busType);
-                const newTarget = getNextAvailableTarget(existingTargets, busType);
-                targetElem.setAttribute("dev", newTarget);
-
-                const addressElem = getSingleOptionalElem(disk, "address");
-                addressElem.remove();
-            }
-
-            const driverElem = disk.getElementsByTagName("driver")[0];
-            if (cache)
-                driverElem.setAttribute("cache", cache);
-        }
-    }
-
-    return s.serializeToString(doc);
-}
 
 export function updateBootOrder(domXml, devices) {
     const s = new XMLSerializer();

diff --git a/src/libvirtApi/domain.js b/src/libvirtApi/domain.js
@@ -48,6 +48,7 @@ import {
 import {
     convertToUnit,
     DOMAINSTATE,
+    getNextAvailableTarget,
     fileDownload,
     getHostDevSourceObject,
     getNodeDevSource,
@@ -64,7 +65,6 @@ import {
 } from '../libvirt-xml-parse.js';
 import {
     updateBootOrder,
-    updateDisk,
     updateMaxMemory,
 } from '../libvirt-xml-update.js';
 import { storagePoolRefresh } from './storagePool.js';
@@ -874,10 +874,45 @@ export function domainStart({ connectionName, id: objPath }) {
     return call(connectionName, objPath, 'org.libvirt.Domain', 'Create', [0], { timeout, type: 'u' });
 }
 
-export function domainUpdateDiskAttributes({ connectionName, objPath, target, readonly, shareable, busType, existingTargets, cache }) {
-    return call(connectionName, objPath, 'org.libvirt.Domain', 'GetXMLDesc', [Enum.VIR_DOMAIN_XML_INACTIVE], { timeout, type: 'u' })
-            .then(domXml => {
-                const updatedXML = updateDisk({ diskTarget: target, domXml, readonly, shareable, busType, existingTargets, cache });
-                return call(connectionName, '/org/libvirt/QEMU', 'org.libvirt.Connect', 'DomainDefineXML', [updatedXML], { timeout, type: 's' });
-            });
+export function domainUpdateDiskAttributes({ connectionName, vmName, target, readonly, shareable, busType, oldBusType, existingTargets, cache }) {
+    const options = { err: "message" };
+    if (connectionName === "system")
+        options.superuser = "try";
+
+    const shareableOption = shareable ? "yes" : "no";
+    const readonlyOption = readonly ? "yes" : "no";
+    let newTarget = target;
+    let addressBus;
+    let addressType;
+    if (busType !== oldBusType) {
+        newTarget = getNextAvailableTarget(existingTargets, busType);
+        // Workaround for https://github.com/virt-manager/virt-manager/issues/430
+        // Until that issue is fixed, we have to change address type and address bus manually
+        if (busType === "virtio")
+            addressType = "pci";
+        if (busType === "usb" || busType === "scsi" || busType === "sata") {
+            // The only allowed bus value is '0' as defined in function qemuValidateDomainDeviceDefAddressDrive at
+            // https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_validate.c
+            addressBus = 0;
+            if (busType === "usb")
+                addressType = "usb";
+            else
+                addressType = "drive";
+        }
+    }
+    let cacheMode = "";
+    if (cache)
+        cacheMode = `,cache=${cache}`;
+
+    const args = [
+        "virt-xml", "-c", `qemu:///${connectionName}`,
+        vmName, "--edit", `target.dev=${target}`, "--disk",
+        `shareable=${shareableOption},readonly=${readonlyOption},target.bus=${busType},target.dev=${newTarget}${cacheMode}`
+    ];
+    if (addressType)
+        args[args.length - 1] += (`,address.type=${addressType}`);
+    if (!isNaN(addressBus)) // addressBus can also be 0
+        args[args.length - 1] += (`,address.bus=${addressBus}`);
+
+    return cockpit.spawn(args, options);
 }