Introduce virt-xml to disk update API

cockpit-project · Apr 4, 2023 · bfe9e9f · bfe9e9f
1 parent 257ff3b
commit bfe9e9f
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 59 deletions.
diff --git a/src/components/vm/disks/diskEdit.jsx b/src/components/vm/disks/diskEdit.jsx
@@ -206,11 +206,12 @@ export class EditDiskModal extends React.Component {
 
         domainUpdateDiskAttributes({
             connectionName: vm.connectionName,
-            objPath: vm.id,
+            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-parse.js b/src/libvirt-xml-parse.js
@@ -399,6 +399,7 @@ export function parseDumpxmlForDisks(devicesElem) {
             const readonlyElem = getSingleOptionalElem(diskElem, 'readonly');
             const shareableElem = getSingleOptionalElem(diskElem, 'shareable');
             const bootElem = getSingleOptionalElem(diskElem, 'boot');
+            const addressElem = getSingleOptionalElem(diskElem, 'address');
 
             const sourceHostElem = sourceElem ? getSingleOptionalElem(sourceElem, 'host') : undefined;
 
@@ -435,6 +436,19 @@ export function parseDumpxmlForDisks(devicesElem) {
                 readonly: !!readonlyElem,
                 shareable: !!shareableElem,
                 removable: targetElem.getAttribute('removable'),
+                address: addressElem
+                    ? {
+                        type: driverElem.getAttribute('type'),
+                        bus: driverElem.getAttribute('bus'),
+                        slot: driverElem.getAttribute('slot'),
+                        function: driverElem.getAttribute('function'),
+                        domain: driverElem.getAttribute('domain'),
+                        multifunction: driverElem.getAttribute('multifunction'),
+                        controller: driverElem.getAttribute('controller'),
+                        target: driverElem.getAttribute('target'),
+                        unit: driverElem.getAttribute('unit')
+                    }
+                    : {},
             };
 
             if (disk.target) {

diff --git a/src/libvirt-xml-update.js b/src/libvirt-xml-update.js
@@ -1,5 +1,4 @@
 import { getDoc, getSingleOptionalElem } from './libvirt-xml-parse.js';
-import { getNextAvailableTarget } from './helpers.js';
 
 export function changeMedia({ domXml, target, eject, file, pool, volume }) {
     const s = new XMLSerializer();
@@ -42,56 +41,6 @@ export function changeMedia({ domXml, target, eject, file, pool, volume }) {
     return s.serializeToString(deviceXml);
 }
 
-export function updateDisk({ domXml, diskTarget, readonly, shareable, busType, existingTargets, cache }) {
-    const s = new XMLSerializer();
-    const doc = getDoc(domXml);
-    const domainElem = doc.firstElementChild;
-    if (!domainElem)
-        return Promise.reject(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();
     const doc = getDoc(domXml);

diff --git a/src/libvirtApi/domain.js b/src/libvirtApi/domain.js
@@ -40,6 +40,7 @@ import {
 } from '../components/create-vm-dialog/uiState.js';
 import {
     DOMAINSTATE,
+    getNextAvailableTarget,
     fileDownload,
     getHostDevSourceObject,
     getNodeDevSource,
@@ -56,7 +57,6 @@ import {
 import {
     changeMedia,
     updateBootOrder,
-    updateDisk,
     updateMaxMemory,
 } from '../libvirt-xml-update.js';
 import { storagePoolRefresh } from './storagePool.js';
@@ -990,10 +990,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);
 }