From c0a51a4aa07b182b1866de1cbb0acd14a67cd8c6 Mon Sep 17 00:00:00 2001 From: Simon Kobyda Date: Fri, 24 Sep 2021 14:58:33 +0200 Subject: [PATCH 1/2] Introduce virt-xml into disk attachement --- src/components/vm/disks/diskAdd.jsx | 2 -- src/libvirt-xml-create.js | 45 ----------------------------- src/libvirtApi/domain.js | 41 +++++++++++++++----------- src/libvirtApi/storageVolume.js | 5 ++-- 4 files changed, 26 insertions(+), 67 deletions(-) diff --git a/src/components/vm/disks/diskAdd.jsx b/src/components/vm/disks/diskAdd.jsx index ba3ddaeb5..ec51fc070 100644 --- a/src/components/vm/disks/diskAdd.jsx +++ b/src/components/vm/disks/diskAdd.jsx @@ -666,7 +666,6 @@ export class AddDiskModalBody extends React.Component { permanent: this.state.permanent, hotplug: this.state.hotplug, vmName: vm.name, - vmId: vm.id, cacheMode: this.state.cacheMode, busType: this.state.busType, serial: clearSerial(this.state.serial) @@ -698,7 +697,6 @@ export class AddDiskModalBody extends React.Component { permanent: this.state.permanent, hotplug: this.state.hotplug, vmName: vm.name, - vmId: vm.id, cacheMode: this.state.cacheMode, shareable: volume && volume.format === "raw" && isVolumeUsed[this.state.existingVolumeName].length > 0, busType: this.state.busType, diff --git a/src/libvirt-xml-create.js b/src/libvirt-xml-create.js index d1747c583..14b7f54f1 100644 --- a/src/libvirt-xml-create.js +++ b/src/libvirt-xml-create.js @@ -1,48 +1,3 @@ -export function getDiskXML(type, file, device, poolName, volumeName, format, target, cacheMode, shareable, busType, serial) { - const doc = document.implementation.createDocument('', '', null); - - const diskElem = doc.createElement('disk'); - diskElem.setAttribute('type', type); - diskElem.setAttribute('device', device); - - const driverElem = doc.createElement('driver'); - driverElem.setAttribute('name', 'qemu'); - if (format && ['qcow2', 'raw'].includes(format)) - driverElem.setAttribute('type', format); - driverElem.setAttribute('cache', cacheMode); - driverElem.setAttribute('discard', 'unmap'); - diskElem.appendChild(driverElem); - - const sourceElem = doc.createElement('source'); - if (type === 'file') { - sourceElem.setAttribute('file', file); - } else { - sourceElem.setAttribute('volume', volumeName); - sourceElem.setAttribute('pool', poolName); - } - diskElem.appendChild(sourceElem); - - const targetElem = doc.createElement('target'); - targetElem.setAttribute('dev', target); - targetElem.setAttribute('bus', busType); - diskElem.appendChild(targetElem); - - if (shareable) { - const shareableElem = doc.createElement('shareable'); - diskElem.appendChild(shareableElem); - } - - if (serial) { - const serialElem = doc.createElement('serial'); - serialElem.appendChild(doc.createTextNode(serial)); - diskElem.appendChild(serialElem); - } - - doc.appendChild(diskElem); - - return new XMLSerializer().serializeToString(doc.documentElement); -} - export function getNetworkXML({ name, forwardMode, device, ipv4, netmask, ipv6, prefix, ipv4DhcpRangeStart, ipv4DhcpRangeEnd, ipv6DhcpRangeStart, ipv6DhcpRangeEnd }) { const doc = document.implementation.createDocument('', '', null); diff --git a/src/libvirtApi/domain.js b/src/libvirtApi/domain.js index edc4cd3c5..0d7ba6f07 100644 --- a/src/libvirtApi/domain.js +++ b/src/libvirtApi/domain.js @@ -31,9 +31,6 @@ import { undefineVm, updateOrAddVm, } from '../actions/store-actions.js'; -import { - getDiskXML, -} from '../libvirt-xml-create.js'; import { setVmCreateInProgress, setVmInstallInProgress, @@ -101,17 +98,6 @@ function buildConsoleVVFile(consoleDetail) { 'fullscreen=0\n'; } -function domainAttachDevice({ connectionName, vmId, permanent, hotplug, xmlDesc }) { - let flags = Enum.VIR_DOMAIN_AFFECT_CURRENT; - if (hotplug) - flags |= Enum.VIR_DOMAIN_AFFECT_LIVE; - if (permanent) - flags |= Enum.VIR_DOMAIN_AFFECT_CONFIG; - - // Error handling is done from the calling side - return call(connectionName, vmId, 'org.libvirt.Domain', 'AttachDevice', [xmlDesc, flags], { timeout, type: 'su' }); -} - export function getPythonPath() { return cockpit.spawn(["/bin/sh", "-c", "command -v /usr/libexec/platform-python || command -v python3 || command -v python"]).then(pyexe => { pythonPath = pyexe.trim() }); } @@ -125,7 +111,6 @@ export function domainAttachDisk({ volumeName, format, target, - vmId, vmName, permanent, hotplug, @@ -134,9 +119,31 @@ export function domainAttachDisk({ busType, serial, }) { - const xmlDesc = getDiskXML(type, file, device, poolName, volumeName, format, target, cacheMode, shareable, busType, serial); + const options = { err: "message" }; + if (connectionName === "system") + options.superuser = "try"; + let define = "--define"; + if (hotplug && !permanent) + define = "--no-define"; + let source = ""; + if (type === 'file') + source = `,source.file=${file}`; + else + source = `,source.pool=${poolName},source.volume=${volumeName}`; + let driverType = ""; + if (format && ['qcow2', 'raw'].includes(format)) + driverType = `,driver.type=${format}`; + let serialNum = ""; + if (serial) + serialNum = `,serial=${serial}`; + const shareableOption = shareable ? "yes" : "no"; - return domainAttachDevice({ connectionName, vmId, permanent, hotplug, xmlDesc }); + const args = ["virt-xml", "-c", `qemu:///${connectionName}`, vmName, "--add-device", "--disk", `type=${type},shareable=${shareableOption},target.bus=${busType},target.dev=${target},driver.name=qemu,driver.discard=unmap,cache=${cacheMode},device=${device}${source}${driverType}${serialNum}`, define]; + + if (hotplug) + args.push("--update"); + + return cockpit.spawn(args, options); } export function domainAttachHostDevices({ connectionName, vmName, live, devices }) { diff --git a/src/libvirtApi/storageVolume.js b/src/libvirtApi/storageVolume.js index 34e9d5c11..2b779d161 100644 --- a/src/libvirtApi/storageVolume.js +++ b/src/libvirtApi/storageVolume.js @@ -46,7 +46,6 @@ export function storageVolumeCreateAndAttach({ size, format, target, - vmId, vmName, permanent, hotplug, @@ -63,8 +62,8 @@ export function storageVolumeCreateAndAttach({ return storagePoolRefresh({ connectionName, objPath: storagePoolPath[0] }); }); }) - .then((volPath) => { - return domainAttachDisk({ connectionName, type: "volume", device: "disk", poolName, volumeName, format, target, vmId, permanent, hotplug, cacheMode, busType, serial }); + .then(() => { + return domainAttachDisk({ connectionName, type: "volume", device: "disk", poolName, volumeName, format, target, vmName, permanent, hotplug, cacheMode, busType, serial }); }); } From 8c396984eadcf4062f23c1285d37b8b8e703bc92 Mon Sep 17 00:00:00 2001 From: Simon Kobyda Date: Wed, 7 Sep 2022 10:53:45 +0200 Subject: [PATCH 2/2] Introduce virt-xml to disk update API --- src/components/vm/disks/diskEdit.jsx | 3 +- src/libvirt-xml-parse.js | 14 ++++++++ src/libvirt-xml-update.js | 51 ---------------------------- src/libvirtApi/domain.js | 49 ++++++++++++++++++++++---- 4 files changed, 58 insertions(+), 59 deletions(-) diff --git a/src/components/vm/disks/diskEdit.jsx b/src/components/vm/disks/diskEdit.jsx index 1466347c0..e585e3f22 100644 --- 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 index f4a5d02f5..f3222984a 100644 --- a/src/libvirt-xml-parse.js +++ b/src/libvirt-xml-parse.js @@ -401,6 +401,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; @@ -437,6 +438,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 index d6254eb2d..1fc7344d9 100644 --- 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 index 0d7ba6f07..d430fe1d1 100644 --- a/src/libvirtApi/domain.js +++ b/src/libvirtApi/domain.js @@ -39,6 +39,7 @@ import { } from '../components/create-vm-dialog/uiState.js'; import { DOMAINSTATE, + getNextAvailableTarget, fileDownload, getHostDevSourceObject, getNodeDevSource, @@ -60,7 +61,6 @@ import { import { changeMedia, updateBootOrder, - updateDisk, updateMaxMemory, } from '../libvirt-xml-update.js'; import { storagePoolRefresh } from './storagePool.js'; @@ -1108,10 +1108,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); }