Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce virt-xml into disk APIs #394

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/components/vm/disks/diskAdd.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion src/components/vm/disks/diskEdit.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down
45 changes: 0 additions & 45 deletions src/libvirt-xml-create.js
Original file line number Diff line number Diff line change
@@ -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);

Expand Down
14 changes: 14 additions & 0 deletions src/libvirt-xml-parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
51 changes: 0 additions & 51 deletions src/libvirt-xml-update.js
Original file line number Diff line number Diff line change
@@ -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();
Expand Down Expand Up @@ -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);
Expand Down
90 changes: 66 additions & 24 deletions src/libvirtApi/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ import {
undefineVm,
updateOrAddVm,
} from '../actions/store-actions.js';
import {
getDiskXML,
} from '../libvirt-xml-create.js';
import {
setVmCreateInProgress,
setVmInstallInProgress,
Expand All @@ -42,6 +39,7 @@ import {
} from '../components/create-vm-dialog/uiState.js';
import {
DOMAINSTATE,
getNextAvailableTarget,
fileDownload,
getHostDevSourceObject,
getNodeDevSource,
Expand All @@ -63,7 +61,6 @@ import {
import {
changeMedia,
updateBootOrder,
updateDisk,
updateMaxMemory,
} from '../libvirt-xml-update.js';
import { storagePoolRefresh } from './storagePool.js';
Expand Down Expand Up @@ -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() });
}
Expand All @@ -125,7 +111,6 @@ export function domainAttachDisk({
volumeName,
format,
target,
vmId,
vmName,
permanent,
hotplug,
Expand All @@ -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";

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 domainAttachDevice({ connectionName, vmId, permanent, hotplug, xmlDesc });
return cockpit.spawn(args, options);
}

export function domainAttachHostDevices({ connectionName, vmName, live, devices }) {
Expand Down Expand Up @@ -1101,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") {
Comment on lines +1126 to +1127
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 2 added lines are not executed by any test. Details

// 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";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details

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);
}
5 changes: 2 additions & 3 deletions src/libvirtApi/storageVolume.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export function storageVolumeCreateAndAttach({
size,
format,
target,
vmId,
vmName,
permanent,
hotplug,
Expand All @@ -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 });
});
}

Expand Down