Re: [libvirt] [test-API][PATCH] Add new cases for testing setSchedulerParametersFlags related API
On 2013年12月12日 16:29, Xuesong Zhang wrote: --- cases/sched_params.conf | 120 +++ repos/domain/sched_params.py | 136 --- repos/domain/sched_params_flag.py | 148 ++ 3 files changed, 330 insertions(+), 74 deletions(-) create mode 100755 cases/sched_params.conf create mode 100644 repos/domain/sched_params_flag.py Both sched_params_flag.py and sched_params.py are used to test vcpu scheduler. Most of their codes are duplicated. setSchedulerParameters() is equivalent to setSchedulerParametersFlags with() flag VIR_DOMAIN_AFFECT_CURRENT So we can only keep sched_params_flag.py. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] Add new case for listAllVolumes and update releated conf
On 2013年12月12日 10:13, codong wrote: listAllVolumes is a new API in RHEL7, so add a new case for it. And add the case to releated conf to check the volumes list. modified: cases/storage_dir.conf modified: cases/storage_dir_vol_resize_delta.conf modified: cases/storage_logical.conf modified: cases/storage_netfs.conf new file: repos/storage/list_volumes.py --- cases/storage_dir.conf | 16 + cases/storage_dir_vol_resize_delta.conf |8 +++ cases/storage_logical.conf | 16 + cases/storage_netfs.conf| 16 + repos/storage/list_volumes.py | 97 +++ 5 files changed, 153 insertions(+), 0 deletions(-) create mode 100644 repos/storage/list_volumes.py The testcase works for the pool of dir type only. If you want to test pool of all type in one testcase, the testcase is not good enough obviously. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH V2] Add blockjob related test functions and cases
On 2013年11月27日 10:56, Jincheng Miao wrote: * repos/domain/blkstatsflags.py * repos/domain/block_iotune.py * repos/domain/block_peek.py * repos/domain/block_resize.py * repos/domain/domain_blkio.py * cases/basic_blockjob.conf V1-V2: Removed diskpath params for block_resize, this hard code disk device is not convenient for users. --- cases/basic_blockjob.conf | 83 + repos/domain/blkstatsflags.py | 63 repos/domain/block_iotune.py | 118 ++ repos/domain/block_peek.py| 69 ++ repos/domain/block_resize.py | 94 repos/domain/domain_blkio.py | 163 ++ 6 files changed, 590 insertions(+) create mode 100644 cases/basic_blockjob.conf create mode 100644 repos/domain/blkstatsflags.py create mode 100644 repos/domain/block_iotune.py create mode 100644 repos/domain/block_peek.py create mode 100644 repos/domain/block_resize.py create mode 100644 repos/domain/domain_blkio.py diff --git a/cases/basic_blockjob.conf b/cases/basic_blockjob.conf new file mode 100644 index 000..21332dc --- /dev/null +++ b/cases/basic_blockjob.conf @@ -0,0 +1,83 @@ +domain:install_linux_cdrom +guestname +$defaultname +guestos +$defaultos +guestarch +$defaultarch +vcpu +$defaultvcpu +memory +$defaultmem +hddriver +$defaulthd +nicdriver +$defaultnic +macaddr +54:52:00:45:c3:8a + +domain:install_linux_check +guestname +$defaultname +virt_type +$defaulthv +hddriver +$defaulthd +nicdriver +$defaultnic + +domain:block_iotune +guestname +$defaultname +bytes_sec +10 +iops_sec +0 + +domain:block_iotune +guestname +$defaultname +bytes_sec +0 +iops_sec +1000 + +domain:block_peek +guestname +$defaultname + +domain:block_peek +guestname +$defaultname + +domain:block_resize +guestname +$defaultname +disksize +1G + +domain:blkstats +guestname +$defaultname + +domain:blkstatsflags +guestname +$defaultname +flags +0 + +domain:domain_blkinfo +guestname +$defaultname The domain:domain_blkinfo testcase failed to be run. error is: Parameter blockdev is required Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH V2] Add blockjob related test functions and cases
On 2013年11月28日 13:25, Guannan Ren wrote: On 2013年11月27日 10:56, Jincheng Miao wrote: * repos/domain/blkstatsflags.py * repos/domain/block_iotune.py * repos/domain/block_peek.py * repos/domain/block_resize.py * repos/domain/domain_blkio.py * cases/basic_blockjob.conf V1-V2: Removed diskpath params for block_resize, this hard code disk device is not convenient for users. --- cases/basic_blockjob.conf | 83 + repos/domain/blkstatsflags.py | 63 repos/domain/block_iotune.py | 118 ++ repos/domain/block_peek.py| 69 ++ repos/domain/block_resize.py | 94 repos/domain/domain_blkio.py | 163 ++ 6 files changed, 590 insertions(+) create mode 100644 cases/basic_blockjob.conf create mode 100644 repos/domain/blkstatsflags.py create mode 100644 repos/domain/block_iotune.py create mode 100644 repos/domain/block_peek.py create mode 100644 repos/domain/block_resize.py create mode 100644 repos/domain/domain_blkio.py diff --git a/cases/basic_blockjob.conf b/cases/basic_blockjob.conf new file mode 100644 index 000..21332dc --- /dev/null +++ b/cases/basic_blockjob.conf @@ -0,0 +1,83 @@ +domain:install_linux_cdrom +guestname +$defaultname +guestos +$defaultos +guestarch +$defaultarch +vcpu +$defaultvcpu +memory +$defaultmem +hddriver +$defaulthd +nicdriver +$defaultnic +macaddr +54:52:00:45:c3:8a + +domain:install_linux_check +guestname +$defaultname +virt_type +$defaulthv +hddriver +$defaulthd +nicdriver +$defaultnic + +domain:block_iotune +guestname +$defaultname +bytes_sec +10 +iops_sec +0 + +domain:block_iotune +guestname +$defaultname +bytes_sec +0 +iops_sec +1000 + +domain:block_peek +guestname +$defaultname + +domain:block_peek +guestname +$defaultname + +domain:block_resize +guestname +$defaultname +disksize +1G + +domain:blkstats +guestname +$defaultname + +domain:blkstatsflags +guestname +$defaultname +flags +0 + +domain:domain_blkinfo +guestname +$defaultname The domain:domain_blkinfo testcase failed to be run. error is: Parameter blockdev is require I know the reason, the fix to the issue can be found in another patch. One thing you should notice, every patch should not break the running of existed testcases. If this patch is based on other patch. you should send the two patches in a set. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-API][PATCH] Fix utils.exec_cmd output problem
On 2013年11月26日 17:17, Jincheng Miao wrote: Thanks for review, yes, I missed this situation: stdout is not the subprocess.PIPE. Since the stderr is always subprocess.PIPE, my another way is err after Popen.communicate(). The patch looks like: --- utils/utils.py | 2 ++--- 1 file changed, 2 insertions(+) diff --git a/utils/utils.py b/utils/utils.py index 147c1ef..d107cbd 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -412,6 +412,8 @@ def exec_cmd(command, sudo=False, cwd=None, infile=None, outfile=None, shell=Fal p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd, stdin=infile, stdout=outfile, stderr=subprocess.PIPE) (out, err) = p.communicate(data) if out == None: # Prevent splitlines() from barfing later on out = +if err != : +out += err return (p.returncode, out.splitlines()) def remote_exec_pexpect(hostname, username, password, cmd): Why to append standard error to standard output. It is not right in semantics. In order to get the standard error if executing command failed, the following change is enough: if out == None: # Prevent splitlines() from barfing later on out = +if p.returncode: +out = err return (p.returncode, out.splitlines()) Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-API][PATCH] Fix utils.exec_cmd output problem
On 2013年11月27日 14:11, Jincheng Miao wrote: - Original Message - Why to append standard error to standard output. It is not right in semantics. I think test-api should replace all commands modules with utils.exec_cmd. For commands modules, it merged stderr with stdout: def getstatusoutput(cmd): Return (status, output) of executing cmd in a shell. import os pipe = os.popen('{ ' + cmd + '; } 21', 'r') So for compatible, utils.exec_cmd should do the same thing. Good idea, I will do the work soon. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-API][PATCH] Fix utils.exec_cmd output problem
On 2013年11月26日 09:32, Jincheng Miao wrote: ping gren - Original Message - As described before, this patch should be : --- utils/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/utils/utils.py b/utils/utils.py index 147c1ef..ec09c33 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -409,9 +409,8 @@ def exec_cmd(command, sudo=False, cwd=None, infile=None, outfile=None, shell=Fal p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd, stdin=infile, stdout=outfile, stderr=subprocess.PIPE) (out, err) = p.communicate(data) -if out == None: -# Prevent splitlines() from barfing later on -out = +if out == : +out = err return (p.returncode, out.splitlines()) def remote_exec_pexpect(hostname, username, password, cmd): -- 1.8.3.1 Sorry I missed your patch. The p.returncode can indicate the result of executing command unless you want the standard error. The subprocess.PIPE can ensure the variable out is always string type, but if the stdout is not the subprocess.PIPE, the variable out possibly be the type of None. so I think it is necessary to use the following code if out == None: out = If you want to get standard error in the case of executing command failure. we need to consider other way. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] Add blockjob related cases
On 2013年11月26日 11:49, Jincheng Miao wrote: Add: * repos/domain/blkstatsflags.py * repos/domain/block_iotune.py * repos/domain/block_peek.py * repos/domain/block_resize.py * repos/domain/domain_blkio.py * cases/basic_blockjob.conf Modify: replace virsh commands to calling api in test function * repos/domain/blkstats.py * repos/domain/domain_blkinfo.py --- cases/basic_blockjob.conf | 87 ++ repos/domain/blkstats.py | 2 - repos/domain/blkstatsflags.py | 63 repos/domain/block_iotune.py | 118 + repos/domain/block_peek.py | 69 + repos/domain/block_resize.py | 88 ++ repos/domain/domain_blkinfo.py | 87 -- repos/domain/domain_blkio.py | 165 + 8 files changed, 639 insertions(+), 40 deletions(-) create mode 100644 cases/basic_blockjob.conf create mode 100644 repos/domain/blkstatsflags.py create mode 100644 repos/domain/block_iotune.py create mode 100644 repos/domain/block_peek.py create mode 100644 repos/domain/block_resize.py create mode 100644 repos/domain/domain_blkio.py diff --git a/cases/basic_blockjob.conf b/cases/basic_blockjob.conf new file mode 100644 index 000..65af2c3 --- /dev/null +++ b/cases/basic_blockjob.conf @@ -0,0 +1,87 @@ +domain:install_linux_cdrom +guestname +$defaultname +guestos +$defaultos +guestarch +$defaultarch +vcpu +$defaultvcpu +memory +$defaultmem +hddriver +$defaulthd +nicdriver +$defaultnic +macaddr +54:52:00:45:c3:8a + +domain:install_linux_check +guestname +$defaultname +virt_type +$defaulthv +hddriver +$defaulthd +nicdriver +$defaultnic + +domain:block_iotune +guestname +$defaultname +bytes_sec +10 +iops_sec +0 + +domain:block_iotune +guestname +$defaultname +bytes_sec +0 +iops_sec +1000 + +domain:block_peek +guestname +$defaultname + +domain:block_peek +guestname +$defaultname + +domain:block_resize +guestname +$defaultname +diskpath +/var/lib/libvirt/images/libvirt-test-api The hardcode is not good. when I tested this testcase, my guest doesn't have such disk in the path +disksize +1G + +domain:blkstats +guestname +$defaultname + +domain:blkstatsflags +guestname +$defaultname +flags +0 + +domain:domain_blkinfo +guestname +$defaultname +blockdev +/var/lib/libvirt/images/libvirt-test-api The same. + +domain:domain_blkio +guestname +$defaultname +weight +500 + +domain:undefine +guestname +$defaultname + +options cleanup=enable Could you remove the trailing spaces in the codes. And please split this big patch into two ones, one for new testcases, the other for the change to the existed testcases. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] add new patch for testing pinemulator API
On 2013年11月22日 14:08, Xuesong Zhang wrote: --- cases/pinemulator.conf | 46 + repos/domain/pinemulator.py | 84 + 2 files changed, 130 insertions(+) create mode 100755 cases/pinemulator.conf create mode 100755 repos/domain/pinemulator.py diff --git a/cases/pinemulator.conf b/cases/pinemulator.conf new file mode 100755 index 000..2f270f3 --- /dev/null +++ b/cases/pinemulator.conf @@ -0,0 +1,46 @@ +domain:install_linux_cdrom +guestname +$defaultname +guestos +$defaultos +guestarch +$defaultarch +vcpu +$defaultvcpu +memory +$defaultmem +hddriver +$defaulthd +nicdriver +$defaultnic +imageformat +qcow2 + +domain:pinemulator +guestname +$defaultname +cpulist +0-2 + +domain:pinemulator +guestname +$defaultname +cpulist +3 + +domain:pinemulator +guestname +$defaultname +cpulist +0,2 + +domain:destroy +guestname +$defaultname + + +domain:undefine +guestname +$defaultname + +options cleanup=enable \ No newline at end of file diff --git a/repos/domain/pinemulator.py b/repos/domain/pinemulator.py new file mode 100755 index 000..8d7b800 --- /dev/null +++ b/repos/domain/pinemulator.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python + Query or change the pinning of domain's emulator threads to + host physical CPUs. + + +import libvirt +from libvirt import libvirtError + +from src import sharedmod +from utils import utils + +required_params = ('guestname', 'cpulist',) +optional_params = {} + +def check_pinemulator(guestname, maxcpu, pininfo_after): +check emulator status of the running virtual machine + + +cmd = cat /var/run/libvirt/qemu/%s.pid % guestname +status, pid = utils.exec_cmd(cmd, shell=True) +if status: +logger.error(failed to get the pid of domain %s % guestname) +return 1 + +cmd = grep Cpus_allowed_list /proc/%s/task/%s/status % (pid[0], pid[0]) +status, output = utils.exec_cmd(cmd, shell=True) +if status: +logger.error(failed to get Cpus_allowed_list) +return 1 + +cpu_allowed_list = output[0] +cpulistcheck = cpu_allowed_list.split('\t')[1] +pininfo_in_process = str(utils.param_to_tuple(cpulistcheck, maxcpu)) + +if cmp(pininfo_in_process, pininfo_after): +logger.error(domain emulator pin failed) +return 1 +else: +logger.info(domain emulator pin successed) +return 0 + + +def pinemulator(params): +Dynamically change the real CPUs which can be allocated to the + emulator process of a domain. This function requires privileged + access to the hypervisor. +global logger +logger = params['logger'] +guestname = params['guestname'] +cpulist = params['cpulist'] + +logger.info(the name of virtual machine is %s % guestname) +logger.info(the given cpulist is %s % cpulist) + +maxcpu = utils.get_host_cpus() +logger.info(%s physical cpu on host % maxcpu) + +cpumap = utils.param_to_tuple(cpulist, maxcpu) +if not cpumap: +logger.error(cpulist: Invalid format) +return 1 + +conn = sharedmod.libvirtobj['conn'] + +try: +domobj = conn.lookupByName(guestname) + +pininfo_original = str(domobj.emulatorPinInfo()) +logger.info(the original emulator pin of the domain is: %s % \ +pininfo_original) + +logger.info(pin domain emulator to host cpu %s % cpulist) +domobj.pinEmulator(cpumap) + +pininfo_after = str(domobj.emulatorPinInfo()) +logger.info(the revised emulator pin of the domain is: %s % \ +pininfo_after) + +ret = check_pinemulator(guestname, maxcpu, pininfo_after) +return ret + +except libvirtError, e: +logger.error(libvirt call failed: + str(e)) +return 1 ACK and pushed Thanks Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/6]qemu: add usb-bot scsi controller support
On 09/10/2013 05:26 PM, Guannan Ren wrote: BZ:https://bugzilla.redhat.com/show_bug.cgi?id=917702 v2: After discussion in BZ, qemu guys hope the usb-bot(+usb-uas) can be supported although the absence of its hot-plug feature. In this patch, libvirt gives an unsupported error in this case. v1: https://www.redhat.com/archives/libvir-list/2013-September/msg00020.html And as the missing of hot-plug feature, the replacement of usb-storage is not a urgent thing. disk attached to usb-storage supports hot-plug/unplug already. Guannan Ren(6) qemu: add usb-bot qemu cap flag qemu: add usb-bot model scsi controller support qemu: add usb-bot support from disks points of view qemu: refactor out function to build scsi device qemu commandline qemu: no hot-plug/unplug support currently for usb-bot tests: add xml2argv test for usb-bot scsi controller docs/formatdomain.html.in | 4 ++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 115 --- src/conf/domain_conf.h| 5 + src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 124 src/qemu/qemu_hotplug.c | 14 ++ src/vmx/vmx.c | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args | 11 +++ tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml | 33 + tests/qemuxml2argvtest.c | 3 +++ 13 files changed, 259 insertions(+), 58 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list An inquiring ping. Shall we support usb-bot right now if its qemu hot-plug feature is not available yet. Gunannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/6] qemu: add usb-bot support from disks points of view
usb-bot only supports 16 luns(0~15) and they must be contiguous, (using lun 0 and 2 without 1 doesn't work). In this case qemu doesn't throw an error, we can not find the lun 2 in guests. So Adding a checking function in libvirt to prevent from this case. --- src/conf/domain_conf.c | 59 src/conf/domain_conf.h | 4 src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 32 ++ 4 files changed, 96 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7fd9422..7a4969e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4302,6 +4302,65 @@ virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, return 0; } +bool +virDomainDiskAttachedToUsbbotLunIsContiguous(virDomainDefPtr def) +{ +size_t i; +int controllerModel; +virBitmapPtr units = NULL; +bool is_set = false; +bool ret = false; + +if (!(units = virBitmapNew(SCSI_CONTROLLER_USB_BOT_MODEL_MAX_LUNS))) +goto cleanup; + +for (i = 0; i def-ndisks; i++) { +virDomainDiskDefPtr disk = def-disks[i]; +int unitValue = disk-info.addr.drive.unit; + +if (disk-bus != VIR_DOMAIN_DISK_BUS_SCSI) +continue; + +controllerModel = +virDomainDeviceFindControllerModel(def, disk-info, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); +if (controllerModel != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) +continue; + +/* usb-bot only supports 16 luns */ +if (unitValue ~0xf) { +virReportError(VIR_ERR_XML_ERROR, + _(The address unit value of disk '%s' is too big), + disk-src); +goto cleanup; +} + +if (virBitmapGetBit(units, unitValue, is_set) == 0 is_set) { +virReportError(VIR_ERR_XML_ERROR, + _(The address unit value of disk '%s' is already used), + disk-src); +goto cleanup; +} + +if (unitValue 0) { +if (virBitmapGetBit(units, unitValue - 1, is_set) == 0 !is_set) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(The address unit value of disk + attached to usb-bot controller is not contiguous)); +goto cleanup; +} +} + +ignore_value(virBitmapSetBit(units, unitValue)); +} + +ret = true; + +cleanup: +virBitmapFree(units); +return ret; +} + static virSecurityLabelDefPtr virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, unsigned int flags) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 11ed18a..cea979f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -781,6 +781,8 @@ typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST } virDomainControllerModelPCI; +# define SCSI_CONTROLLER_USB_BOT_MODEL_MAX_LUNS 16 + enum virDomainControllerModelSCSI { VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC, @@ -2365,6 +2367,8 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, virDomainDiskDefPtr def); +bool virDomainDiskAttachedToUsbbotLunIsContiguous(virDomainDefPtr def); + virDomainDiskDefPtr virDomainDiskRemove(virDomainDefPtr def, size_t i); virDomainDiskDefPtr diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..c1f7da5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -179,6 +179,7 @@ virDomainDeviceFindControllerModel; virDomainDeviceInfoCopy; virDomainDeviceInfoIterate; virDomainDeviceTypeToString; +virDomainDiskAttachedToUsbbotLunIsContiguous; virDomainDiskBusTypeToString; virDomainDiskCacheTypeFromString; virDomainDiskCacheTypeToString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f40c050..1a6accd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4316,6 +4316,32 @@ qemuBuildDriveDevStr(virDomainDefPtr def, disk-info.addr.drive.controller, disk-info.addr.drive.bus, disk-info.addr.drive.unit); +} else if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { +if (disk-info.addr.drive.target != 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(target must be 0 for controller + model 'usb-bot')); +goto error; +} + +if (disk-device != VIR_DOMAIN_DISK_DEVICE_LUN) { +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { +if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM) +
[libvirt] [PATCH v2 4/6] qemu: refactor out function to build scsi device qemu commandline
--- src/qemu/qemu_command.c | 124 +++- 1 file changed, 48 insertions(+), 76 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1a6accd..aa91f57 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4153,6 +4153,40 @@ error: return NULL; } +static int +qemuBuildSCSIDriveDevStr(int controllerModel, + virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps, + virBufferPtr opt) +{ +if (disk-device != VIR_DOMAIN_DISK_DEVICE_LUN) { +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { +if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM) +virBufferAddLit(opt, scsi-cd); +else +virBufferAddLit(opt, scsi-hd); +} else { +virBufferAddLit(opt, scsi-disk); +} +} else { +virBufferAddLit(opt, scsi-block); +} + + +if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) +virBufferAsprintf(opt, ,bus=scsi%d.%d,scsi-id=%d, + disk-info.addr.drive.controller, + disk-info.addr.drive.bus, + disk-info.addr.drive.unit); +else +virBufferAsprintf(opt, ,bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d, + disk-info.addr.drive.controller, + disk-info.addr.drive.bus, + disk-info.addr.drive.target, + disk-info.addr.drive.unit); +return 0; +} + char * qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskDefPtr disk, @@ -4291,94 +4325,32 @@ qemuBuildDriveDevStr(virDomainDefPtr def, if ((qemuSetScsiControllerModel(def, qemuCaps, controllerModel)) 0) goto error; -if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { +if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC || +controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { if (disk-info.addr.drive.target != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(target must be 0 for controller model 'lsilogic')); goto error; } - -if (disk-device == VIR_DOMAIN_DISK_DEVICE_LUN) { -virBufferAddLit(opt, scsi-block); -} else { -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { -if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM) -virBufferAddLit(opt, scsi-cd); -else -virBufferAddLit(opt, scsi-hd); -} else { -virBufferAddLit(opt, scsi-disk); -} -} - -virBufferAsprintf(opt, ,bus=scsi%d.%d,scsi-id=%d, - disk-info.addr.drive.controller, - disk-info.addr.drive.bus, - disk-info.addr.drive.unit); -} else if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { -if (disk-info.addr.drive.target != 0) { +} else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { +if (disk-info.addr.drive.target 7) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(target must be 0 for controller - model 'usb-bot')); + _(This QEMU doesn't support target + greater than 7)); goto error; } -if (disk-device != VIR_DOMAIN_DISK_DEVICE_LUN) { -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { -if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM) -virBufferAddLit(opt, scsi-cd); -else -virBufferAddLit(opt, scsi-hd); -} else { -virBufferAddLit(opt, scsi-disk); -} -} else { -virBufferAddLit(opt, scsi-block); -} - -virBufferAsprintf(opt, ,bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d, - disk-info.addr.drive.controller, - disk-info.addr.drive.bus, - disk-info.addr.drive.target, - disk-info.addr.drive.unit); -} else { -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { -if (disk-info.addr.drive.target 7) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(This QEMU doesn't support target - greater than 7)); -
[libvirt] [PATCH v2 2/6] qemu: add usb-bot model scsi controller
usb-bot is SCSI HBA which support only one SCSI target with ID 0. we can create one or more SCSI devices connected to it with -device as its luns. For usb-bot the limit is 15 luns. The difference from other SCSI controllers is that usb-bot needs usb-bus support. That means usb-bot is required to be attached to a existing USB controller. libvirt xml example: devices ... controller type='usb' index='0' /controller controller type='scsi' index='0' model='usb-bot' address type='usb' bus='0' port='1'/ /controller ... /devices QEMU commandline should be: -device piix3-usb-uhci,id=usb \ -device usb-bot,id=scsi0,bus=usb.0,port=1 --- docs/formatdomain.html.in | 4 ++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 52 +-- src/conf/domain_conf.h| 1 + src/qemu/qemu_command.c | 16 + src/vmx/vmx.c | 3 ++- 6 files changed, 72 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f8bfe0b..07887f2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2417,8 +2417,8 @@ control how many devices can be connected through the controller. A scsi controller has an optional attribute codemodel/code, which is one of auto, buslogic, - ibmvscsi, lsilogic, lsisas1068, lsisas1078, virtio-scsi or - vmpvscsi. A usb controller has an optional attribute + ibmvscsi, lsilogic, lsisas1068, lsisas1078, virtio-scsi, + vmpvscsi or usb-bot. A usb controller has an optional attribute codemodel/code, which is one of piix3-uhci, piix4-uhci, ehci, ich9-ehci1, ich9-uhci1, ich9-uhci2, ich9-uhci3, vt82c686b-uhci, pci-ohci or nec-xhci. Additionally, diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ecd3a42..7ce9888 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1537,6 +1537,7 @@ valueibmvscsi/value valuevirtio-scsi/value valuelsisas1078/value + valueusb-bot/value /choice /attribute /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aed2a9d..7fd9422 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -326,7 +326,8 @@ VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAS vmpvscsi, ibmvscsi, virtio-scsi, - lsisas1078); + lsisas1078, + usb-bot); VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, piix3-uhci, @@ -5772,6 +5773,17 @@ virDomainControllerDefParseXML(xmlNodePtr node, goto error; switch (def-type) { +case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: +if (def-model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { +if (def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE +def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(usb-bot mode of scsi controller requires + address of type 'usb')); +goto error; +} +} +break; case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: { char *ports = virXMLPropString(node, ports); if (ports) { @@ -5863,7 +5875,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO -def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { +def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI +def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Controllers must use the 'pci' address type)); goto error; @@ -13803,6 +13816,38 @@ virDomainDefMaybeAddSmartcardController(virDomainDefPtr def) return 0; } +static int +virDomainDefMaybeAddUSBcontroller(virDomainDefPtr def) +{ +size_t i; +int maxController = -1; + +for (i = 0; i def-ncontrollers; i++) { +virDomainControllerDefPtr cont = def-controllers[i]; + +if (cont-type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) +continue; + +if (cont-model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) +continue; + +if ((int)cont-info.addr.usb.bus maxController) +maxController = cont-info.addr.usb.bus; +} + +if (maxController == -1) +return 0; + +for (i = 0; i = maxController; i++) { +if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_USB, +
[libvirt] [PATCH v2 0/6]qemu: add usb-bot scsi controller support
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=917702 v2: After discussion in BZ, qemu guys hope the usb-bot(+usb-uas) can be supported although the absence of its hot-plug feature. In this patch, libvirt gives an unsupported error in this case. v1: https://www.redhat.com/archives/libvir-list/2013-September/msg00020.html And as the missing of hot-plug feature, the replacement of usb-storage is not a urgent thing. disk attached to usb-storage supports hot-plug/unplug already. Guannan Ren(6) qemu: add usb-bot qemu cap flag qemu: add usb-bot model scsi controller support qemu: add usb-bot support from disks points of view qemu: refactor out function to build scsi device qemu commandline qemu: no hot-plug/unplug support currently for usb-bot tests: add xml2argv test for usb-bot scsi controller docs/formatdomain.html.in | 4 ++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 115 --- src/conf/domain_conf.h| 5 + src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 124 src/qemu/qemu_hotplug.c | 14 ++ src/vmx/vmx.c | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args | 11 +++ tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml | 33 + tests/qemuxml2argvtest.c | 3 +++ 13 files changed, 259 insertions(+), 58 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/6] qemu: no hot-plug/unplug support currently for usb-bot
--- src/conf/domain_conf.c | 4 +++- src/qemu/qemu_hotplug.c | 14 ++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7a4969e..60ca298 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17118,7 +17118,9 @@ virDomainDeviceIsUSB(virDomainDeviceDefPtr dev) (t == VIR_DOMAIN_DEVICE_HUB dev-data.hub-type == VIR_DOMAIN_HUB_TYPE_USB) || (t == VIR_DOMAIN_DEVICE_REDIRDEV - dev-data.redirdev-bus == VIR_DOMAIN_REDIRDEV_BUS_USB)) + dev-data.redirdev-bus == VIR_DOMAIN_REDIRDEV_BUS_USB) || +(t == VIR_DOMAIN_DEVICE_CONTROLLER + dev-data.controller-model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT)) return true; return false; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6cdee44..b6e3c4c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -365,6 +365,13 @@ int qemuDomainAttachPciControllerDevice(virQEMUDriverPtr driver, return -1; } +if (controller-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI +controller-model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(device usb-bot hotplug unsupported currently)); +goto cleanup; +} + if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuDomainPCIAddressEnsureAddr(priv-pciaddrs, controller-info) 0) goto cleanup; @@ -3009,6 +3016,13 @@ int qemuDomainDetachPciControllerDevice(virQEMUDriverPtr driver, detach = vm-def-controllers[idx]; +if (detach-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI +detach-model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(device usb-bot cannot be detached currently)); +goto cleanup; +} + if (!virDomainDeviceAddressIsValid(detach-info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { virReportError(VIR_ERR_OPERATION_FAILED, %s, -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 6/6] tests: add xml2argv test for usb-bot scsi controller
--- .../qemuxml2argv-disk-scsi-usbbot.args | 11 .../qemuxml2argv-disk-scsi-usbbot.xml | 33 ++ tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 47 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args new file mode 100644 index 000..572a8b0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args @@ -0,0 +1,11 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device usb-bot,id=scsi0 -usb \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-scsi0-0-0-0 \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-drive file=/tmp/scsidisk.img,if=none,id=drive-scsi0-0-0-1 \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=1,\ +drive=drive-scsi0-0-0-1,id=scsi0-0-0-1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml new file mode 100644 index 000..4be405f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml @@ -0,0 +1,33 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='sda' bus='scsi'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +disk type='file' device='disk' + source file='/tmp/scsidisk.img'/ + target dev='sdb' bus='scsi'/ + address type='drive' controller='0' bus='0' target='0' unit='1'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='scsi' index='0' model='usb-bot'/ +controller type='pci' index='0' model='pci-root'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 15d3095..a904c71 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -552,6 +552,9 @@ mymain(void) DO_TEST(disk-scsi-device, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); +DO_TEST(disk-scsi-usbbot, +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, +QEMU_CAPS_DEVICE_USB_BOT); DO_TEST(disk-scsi-device-auto, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/6] qemu: add usb-bot qemu cap flag
QEMU_CAPS_DEVICE_USB_BOT /* -device usb-bot */ --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d94188a..5cd7d45 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -241,6 +241,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, usb-storage, /* 155 */ usb-storage.removable, virtio-mmio, + usb-bot, ); struct _virQEMUCaps { @@ -1379,6 +1380,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { vmware-svga, QEMU_CAPS_DEVICE_VMWARE_SVGA }, { usb-serial, QEMU_CAPS_DEVICE_USB_SERIAL }, { usb-net, QEMU_CAPS_DEVICE_USB_NET }, +{ usb-bot, QEMU_CAPS_DEVICE_USB_BOT }, { virtio-rng-pci, QEMU_CAPS_DEVICE_VIRTIO_RNG }, { virtio-rng-s390, QEMU_CAPS_DEVICE_VIRTIO_RNG }, { virtio-rng-ccw, QEMU_CAPS_DEVICE_VIRTIO_RNG }, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f3c8fa8..1e17f4f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -196,6 +196,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_USB_STORAGE = 155, /* -device usb-storage */ QEMU_CAPS_USB_STORAGE_REMOVABLE = 156, /* usb-storage.removable */ QEMU_CAPS_DEVICE_VIRTIO_MMIO = 157, /* -device virtio-mmio */ +QEMU_CAPS_DEVICE_USB_BOT = 158, /* -device usb-bot */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] qemu: refactor out function to build scsi device qemu commandline
--- src/qemu/qemu_command.c | 124 +++- 1 file changed, 48 insertions(+), 76 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f38e98f..e46baaf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4094,6 +4094,40 @@ error: return NULL; } +static int +qemuBuildSCSIDriveDevStr(int controllerModel, + virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps, + virBufferPtr opt) +{ +if (disk-device != VIR_DOMAIN_DISK_DEVICE_LUN) { +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { +if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM) +virBufferAddLit(opt, scsi-cd); +else +virBufferAddLit(opt, scsi-hd); +} else { +virBufferAddLit(opt, scsi-disk); +} +} else { +virBufferAddLit(opt, scsi-block); +} + + +if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) +virBufferAsprintf(opt, ,bus=scsi%d.%d,scsi-id=%d, + disk-info.addr.drive.controller, + disk-info.addr.drive.bus, + disk-info.addr.drive.unit); +else +virBufferAsprintf(opt, ,bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d, + disk-info.addr.drive.controller, + disk-info.addr.drive.bus, + disk-info.addr.drive.target, + disk-info.addr.drive.unit); +return 0; +} + char * qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskDefPtr disk, @@ -4232,94 +4266,32 @@ qemuBuildDriveDevStr(virDomainDefPtr def, if ((qemuSetScsiControllerModel(def, qemuCaps, controllerModel)) 0) goto error; -if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { +if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC || +controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { if (disk-info.addr.drive.target != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(target must be 0 for controller model 'lsilogic')); goto error; } - -if (disk-device == VIR_DOMAIN_DISK_DEVICE_LUN) { -virBufferAddLit(opt, scsi-block); -} else { -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { -if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM) -virBufferAddLit(opt, scsi-cd); -else -virBufferAddLit(opt, scsi-hd); -} else { -virBufferAddLit(opt, scsi-disk); -} -} - -virBufferAsprintf(opt, ,bus=scsi%d.%d,scsi-id=%d, - disk-info.addr.drive.controller, - disk-info.addr.drive.bus, - disk-info.addr.drive.unit); -} else if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { -if (disk-info.addr.drive.target != 0) { +} else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { +if (disk-info.addr.drive.target 7) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(target must be 0 for controller - model 'usb-bot')); + _(This QEMU doesn't support target + greater than 7)); goto error; } -if (disk-device != VIR_DOMAIN_DISK_DEVICE_LUN) { -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { -if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM) -virBufferAddLit(opt, scsi-cd); -else -virBufferAddLit(opt, scsi-hd); -} else { -virBufferAddLit(opt, scsi-disk); -} -} else { -virBufferAddLit(opt, scsi-block); -} - -virBufferAsprintf(opt, ,bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d, - disk-info.addr.drive.controller, - disk-info.addr.drive.bus, - disk-info.addr.drive.target, - disk-info.addr.drive.unit); -} else { -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { -if (disk-info.addr.drive.target 7) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(This QEMU doesn't support target - greater than 7)); -
[libvirt] [PATCH 0/5]qemu: add usb-bot scsi controller support
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=917702 qemu patch:http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg02200.html These patch aims to add usb-bot SCSI controller support for libvirt. As usb-storage libvirt already supported, usb-bot only supports one SCSI target with SCSI ID 0. The difference is that usb-storage creates SCSI HBA and additionaly either a scsi-disk or a scsi-generic device, usb-bot only creates the SCSI HBA. we can attach a SCSI device to it with another -device. usb-bot supports a single target and a number of luns. The limit is 15 luns (0~15 LUN ID) and they must be contiguous(using lun 0 and 2 without 1 doesn't work). Athought usb-bot is a SCSI controller it needs to be attached to a exsiting USB controller for work. So it has to use address of usb type. Libvirt XML: devices ... disk type='file' device='disk' driver name='qemu' type='raw'/ source file='/var/lib/libvirt/images/disk.qcow2'/ target dev='sdc' bus='scsi'/ address type='drive' controller='0' bus='0' target='0' unit='0'/ /disk controller type='usb' index='0'/ controller type='scsi' index='0' model='usb-bot' address type='usb' bus='0' port='1'/ /controller ... /devices The QEMU commandline: qemu ${other_vm_args} \ -device piix3-usb-uhci,id=usb \ -device usb-bot,id=scsi0,bus=usb.0,port=1 \ -drive file=/var/lib/libvirt/images/disk.qcow2,if=none,id=drive-scsi0-0-0-0,format=raw \ -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 As the usb-storage creates scsi disk automatically which doesn't let you set scsi-disk properties such as vendor, product, wwn, channel, scsi-id and lun. So QEMU guys prefer usb-bot to usb-storage. So this is the first part of the whole work. Next step will replace usb-storage with usb-bot when disk in xml uses usb bus like disk ... ... target bus='usb' /disk Guannan Ren(5) qemu: add usb-bot qemu cap flag qemu: add usb-bot model scsi controller support qemu: add usb-bot support from disks points of view qemu: refactor out function to build scsi device qemu commandline tests: add xml2argv test for usb-bot scsi controller docs/formatdomain.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 111 +++-- src/conf/domain_conf.h| 5 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 124 ++--- src/vmx/vmx.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args | 10 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml | 33 tests/qemuxml2argvtest.c | 3 ++ 12 files changed, 242 insertions(+), 57 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] qemu: add usb-bot qemu cap flag
QEMU_CAPS_DEVICE_USB_BOT /* -device usb-bot */ --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7888e2d..6a775ee 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -237,6 +237,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, dmi-to-pci-bridge, i440fx-pci-hole64-size, q35-pci-hole64-size, + + usb-bot, /* 155 */ ); struct _virQEMUCaps { @@ -1375,6 +1377,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { vmware-svga, QEMU_CAPS_DEVICE_VMWARE_SVGA }, { usb-serial, QEMU_CAPS_DEVICE_USB_SERIAL }, { usb-net, QEMU_CAPS_DEVICE_USB_NET }, +{ usb-bot, QEMU_CAPS_DEVICE_USB_BOT }, { virtio-rng-pci, QEMU_CAPS_DEVICE_VIRTIO_RNG }, { virtio-rng-s390, QEMU_CAPS_DEVICE_VIRTIO_RNG }, { virtio-rng-ccw, QEMU_CAPS_DEVICE_VIRTIO_RNG }, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 69f3395..67746a4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -193,6 +193,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */ QEMU_CAPS_I440FX_PCI_HOLE64_SIZE = 153, /* i440FX-pcihost.pci-hole64-size */ QEMU_CAPS_Q35_PCI_HOLE64_SIZE = 154, /* q35-pcihost.pci-hole64-size */ +QEMU_CAPS_DEVICE_USB_BOT = 155, /* -device usb-bot */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] tests: add xml2argv test for usb-bot scsi controller
--- .../qemuxml2argv-disk-scsi-usbbot.args | 10 +++ .../qemuxml2argv-disk-scsi-usbbot.xml | 33 ++ tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 46 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args new file mode 100644 index 000..09e6d57 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device usb-bot,id=scsi0 \ +-usb -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-scsi0-0-0-0 \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-drive file=/tmp/scsidisk.img,if=none,id=drive-scsi0-0-0-1 \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=1,\ +drive=drive-scsi0-0-0-1,id=scsi0-0-0-1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml new file mode 100644 index 000..4be405f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml @@ -0,0 +1,33 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='sda' bus='scsi'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +disk type='file' device='disk' + source file='/tmp/scsidisk.img'/ + target dev='sdb' bus='scsi'/ + address type='drive' controller='0' bus='0' target='0' unit='1'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='scsi' index='0' model='usb-bot'/ +controller type='pci' index='0' model='pci-root'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4e3508b..29de992 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -548,6 +548,9 @@ mymain(void) DO_TEST(disk-scsi-device, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); +DO_TEST(disk-scsi-usbbot, +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, +QEMU_CAPS_DEVICE_USB_BOT); DO_TEST(disk-scsi-device-auto, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] qemu: add usb-bot model scsi controller support
usb-bot is SCSI HBA which support only one SCSI target with ID 0. we can create one or more SCSI devices connected to it with -device as its luns. For usb-bot the limit is 15 luns. The difference from other SCSI controllers is that usb-bot needs usb-bus support. That means usb-bot is required to be attached to a existing USB controller. libvirt xml example: devices ... controller type='usb' index='0' /controller controller type='scsi' index='0' model='usb-bot' address type='usb' bus='0' port='1'/ /controller ... /devices QEMU commandline should be: -device piix3-usb-uhci,id=usb \ -device usb-bot,id=scsi0,bus=usb.0,port=1 --- docs/formatdomain.html.in | 4 ++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 52 +-- src/conf/domain_conf.h| 1 + src/qemu/qemu_command.c | 16 + src/vmx/vmx.c | 3 ++- 6 files changed, 72 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index cce179d..274acf4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2413,8 +2413,8 @@ control how many devices can be connected through the controller. A scsi controller has an optional attribute codemodel/code, which is one of auto, buslogic, - ibmvscsi, lsilogic, lsisas1068, lsisas1078, virtio-scsi or - vmpvscsi. A usb controller has an optional attribute + ibmvscsi, lsilogic, lsisas1068, lsisas1078, virtio-scsi, + vmpvscsi or usb-bot. A usb controller has an optional attribute codemodel/code, which is one of piix3-uhci, piix4-uhci, ehci, ich9-ehci1, ich9-uhci1, ich9-uhci2, ich9-uhci3, vt82c686b-uhci, pci-ohci or nec-xhci. Additionally, diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6978dc7..d748d68 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1509,6 +1509,7 @@ valueibmvscsi/value valuevirtio-scsi/value valuelsisas1078/value + valueusb-bot/value /choice /attribute /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f8fbf79..545c157 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -324,7 +324,8 @@ VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAS vmpvscsi, ibmvscsi, virtio-scsi, - lsisas1078); + lsisas1078, + usb-bot); VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, piix3-uhci, @@ -5736,6 +5737,17 @@ virDomainControllerDefParseXML(xmlNodePtr node, goto error; switch (def-type) { +case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: +if (def-model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { +if (def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE +def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(usb-bot mode of scsi controller requires + address of type 'usb')); +goto error; +} +} +break; case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: { char *ports = virXMLPropString(node, ports); if (ports) { @@ -5826,7 +5838,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 -def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { +def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI +def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Controllers must use the 'pci' address type)); goto error; @@ -13778,6 +13791,38 @@ virDomainDefMaybeAddSmartcardController(virDomainDefPtr def) return 0; } +static int +virDomainDefMaybeAddUSBcontroller(virDomainDefPtr def) +{ +size_t i; +int maxController = -1; + +for (i = 0; i def-ncontrollers; i++) { +virDomainControllerDefPtr cont = def-controllers[i]; + +if (cont-type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) +continue; + +if (cont-model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) +continue; + +if ((int)cont-info.addr.usb.bus maxController) +maxController = cont-info.addr.usb.bus; +} + +if (maxController == -1) +return 0; + +for (i = 0; i = maxController; i++) { +if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_USB, +
[libvirt] [PATCH 3/5] qemu: add usb-bot support from disks points of view
usb-bot only supports 16 luns(0~15) and they must be contiguous, (using lun 0 and 2 without 1 doesn't work). In this case qemu doesn't throw an error, we can not find the lun 2 in guests. So Adding a checking function in libvirt to prevent from this case. --- src/conf/domain_conf.c | 59 src/conf/domain_conf.h | 4 src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 32 ++ 4 files changed, 96 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 545c157..84dfe25 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4287,6 +4287,65 @@ virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, return 0; } +bool +virDomainDiskAttachedToUsbbotLunIsContiguous(virDomainDefPtr def) +{ +size_t i; +int controllerModel; +virBitmapPtr units = NULL; +bool is_set = false; +bool ret = false; + +if (!(units = virBitmapNew(SCSI_CONTROLLER_USB_BOT_MODEL_MAX_LUNS))) +goto cleanup; + +for (i = 0; i def-ndisks; i++) { +virDomainDiskDefPtr disk = def-disks[i]; +int unitValue = disk-info.addr.drive.unit; + +if (disk-bus != VIR_DOMAIN_DISK_BUS_SCSI) +continue; + +controllerModel = +virDomainDeviceFindControllerModel(def, disk-info, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); +if (controllerModel != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) +continue; + +/* usb-bot only supports 16 luns */ +if (unitValue ~0xf) { +virReportError(VIR_ERR_XML_ERROR, + _(The address unit value of disk '%s' is too big), + disk-src); +goto cleanup; +} + +if (virBitmapGetBit(units, unitValue, is_set) == 0 is_set) { +virReportError(VIR_ERR_XML_ERROR, + _(The address unit value of disk '%s' is already used), + disk-src); +goto cleanup; +} + +if (unitValue 0) { +if (virBitmapGetBit(units, unitValue - 1, is_set) == 0 !is_set) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(The address unit value of disk + attached to usb-bot controller is not contiguous)); +goto cleanup; +} +} + +ignore_value(virBitmapSetBit(units, unitValue)); +} + +ret = true; + +cleanup: +virBitmapFree(units); +return ret; +} + static virSecurityLabelDefPtr virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, unsigned int flags) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3e69f84..9e2f477 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -778,6 +778,8 @@ typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST } virDomainControllerModelPCI; +# define SCSI_CONTROLLER_USB_BOT_MODEL_MAX_LUNS 16 + enum virDomainControllerModelSCSI { VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC, @@ -2362,6 +2364,8 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, virDomainDiskDefPtr def); +bool virDomainDiskAttachedToUsbbotLunIsContiguous(virDomainDefPtr def); + virDomainDiskDefPtr virDomainDiskRemove(virDomainDefPtr def, size_t i); virDomainDiskDefPtr diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..c1f7da5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -179,6 +179,7 @@ virDomainDeviceFindControllerModel; virDomainDeviceInfoCopy; virDomainDeviceInfoIterate; virDomainDeviceTypeToString; +virDomainDiskAttachedToUsbbotLunIsContiguous; virDomainDiskBusTypeToString; virDomainDiskCacheTypeFromString; virDomainDiskCacheTypeToString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5f08a72..f38e98f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4257,6 +4257,32 @@ qemuBuildDriveDevStr(virDomainDefPtr def, disk-info.addr.drive.controller, disk-info.addr.drive.bus, disk-info.addr.drive.unit); +} else if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { +if (disk-info.addr.drive.target != 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(target must be 0 for controller + model 'usb-bot')); +goto error; +} + +if (disk-device != VIR_DOMAIN_DISK_DEVICE_LUN) { +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { +if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM) +
Re: [libvirt] [Qemu-devel] [PATCH 3/5] qemu: add usb-bot support from disks points of view
On 09/02/2013 08:57 PM, Daniel P. Berrange wrote: On Mon, Sep 02, 2013 at 05:38:42PM +0800, Guannan Ren wrote: usb-bot only supports 16 luns(0~15) and they must be contiguous, (using lun 0 and 2 without 1 doesn't work). In this case qemu doesn't throw an error, we can not find the lun 2 in guests. So Adding a checking function in libvirt to prevent from this case. Hmm, this seems like a problematic restriction. How does this work if we start off a guest with 3 disks attached to the usb-bot SCSI controller. Then hot-unplug the 2nd disk. Currently, the qemu will throw out an error when I hot-unplug any of disks attached to usb-bot controller. { execute: device_del, arguments: { id: scsi1-0-0-1}} {error: {class: GenericError, desc: Bus 'scsi1.0' does not support hotplugging}} Libvirt will report an error: # virsh detach-disk rhel64qcow2 sdd --current error: Failed to detach disk error: internal error: unable to execute QEMU command 'device_del': Bus 'scsi1.0' does not support hotplugging Next time we need will need to start with with LUNs 0 and 2 populated only, otherwise we have ABI change upon migrate or save/restore. I think this restriction about contiguous luns must be removed if this device is to be usable with multiple luns. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: fix return value error of cpu-stats
virsh cpu-stats guest --start 0 --count 3 It outputs right but the return value is 1 rather than 0 echo $? 1 Found by running libvirt-autotest ./run -t libvirt --tests virsh_cpu_stats --- tools/virsh-domain.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b29f934..bcf495c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6350,7 +6350,9 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) if (!nparams) { vshPrint(ctl, %s, _(No per-CPU stats available)); -goto do_show_total; +if (show_total) +goto do_show_total; +goto cleanup; } if (VIR_ALLOC_N(params, nparams * MIN(show_count, 128)) 0) @@ -6389,10 +6391,12 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) } VIR_FREE(params); -do_show_total: -if (!show_total) +if (!show_total) { +ret = true; goto cleanup; +} +do_show_total: /* get supported num of parameter for total statistics */ if ((nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, flags)) 0) goto failed_stats; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Test for object identity when checking for None in Python
On 08/22/2013 09:56 PM, Claudio Bley wrote: Consistently use is or is not to compare variables to None, because doing so is preferrable, as per PEP 8 (http://www.python.org/dev/peps/pep-0008/#programming-recommendations): Comparisons to singletons like None should always be done with is or is not, never the equality operators. Signed-off-by: Claudio Bley cb...@av-test.de --- Purely mechanical change, using: find . -name '*.py' -exec sed -i -e 's,[ \t][ \t]*!=[ \t][ \t]*None, is not None,g' '{}' \+ find . -name '*.py' -exec sed -i -e 's,[ \t][ \t]*==[ \t][ \t]*None, is None,g' '{}' \+ This change makes sense. ACK Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util:report diskchain as broken if without enough permission
When backing files of a disk file are stored in NFS shared folder, qemu process requires at least the read permission. But in some rare situation, these backing files can not even be read If so, we should treat the diskchains as broken. This patch add a checking for this, report negative in such case. --- src/util/virstoragefile.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0b9cec3..3dee7ab 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1127,7 +1127,11 @@ virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, /* Break if no backing store or backing store is not file */ if (!tmp-backingStoreRaw) break; - if (!tmp-backingStore) { + + if (tmp-backingStoreIsFile) { + if (!tmp-backingMeta) + goto error; + } else if (!tmp-backingStore) { if (VIR_STRDUP(*brokenFile, tmp-backingStoreRaw) 0) goto error; break; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util:report diskchain as broken if without enough permission
On 08/21/2013 03:57 AM, Eric Blake wrote: On 08/20/2013 01:54 AM, Guannan Ren wrote: When backing files of a disk file are stored in NFS shared folder, qemu process requires at least the read permission. But in some rare situation, these backing files can not even be read If so, we should treat the diskchains as broken. This patch add a checking for this, report negative in such case. We have been historically bitten by changes to this code. I think we need to patch tests/virstoragetest.c (and/or add a new test) before accepting this change, to make sure that we are testing the behavior we want. Okay, will do it soon. Thanks -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/4] storage: report error rather than warning if backing files doesn't exist
On 07/17/2013 05:41 PM, Daniel P. Berrange wrote: On Wed, Jul 17, 2013 at 10:08:55AM +0800, Guannan Ren wrote: On 07/16/2013 04:40 PM, Daniel P. Berrange wrote: On Tue, Jul 16, 2013 at 08:14:54AM +0200, Martin Kletzander wrote: On 07/15/2013 05:31 PM, Ján Tomko wrote: On 07/02/2013 11:35 AM, Guannan Ren wrote: If one of backing files for disk source doesn't exist, the guest will not be able to find and use the disk even though the disk still exists in guest xml definition. So reporting an error make more sense. Adding virFileAccessibleAs() to check if the backing file described in disk meta exist in real path. If not, report error. the uid and gid arguments don't have so much meannings for F_OK, so give 0 for them. --- src/util/virstoragefile.c | 23 +++ tests/virstoragetest.c| 16 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 27aa4fe..cb61e5b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -870,14 +877,10 @@ virStorageFileGetMetadataInternal(const char *path, !!directory, backing, meta-directory, meta-backingStore) 0) { -/* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ -meta-backingStoreIsFile = false; -backingFormat = VIR_STORAGE_FILE_NONE; -VIR_WARN(Backing file '%s' of image '%s' is missing., - meta-backingStoreRaw, path); - +VIR_ERROR(_(Backing file '%s' of image '%s' is missing.), + meta-backingStoreRaw, path); +VIR_FREE(backing); +goto cleanup; } } VIR_FREE(backing); This change means you won't be able to start the pool if one of the files is missing a backing file. I've forwarded a patch [1] from/for [2] that ignores missing files on pool start and there is a bug [3] requesting that we ignore other files as well. I feel like this is going in the other direction. Wouldn't it be enough to check for them on domain start-up and leave the pool running even if one of those volumes doesn't have an existing backing file? How about making it configurable for the pool? There are definitely some users who want the pool to reflect actual info after pool-refresh. I don't think this needs to be configurable. The pool should show *every* single file, regardless of whether the file has a broken backing file. We shouldn't be trying to second guess what the user wants to do with a image with broken backing file. Just expose as much info as we have and let them deal with the problem. Daniel, How about for guest cold bootup in this situation where one of its disks has a broken file we should throw an error out or give a warn in the log as what it is doing currently. If we can't access the file on guest boot, then we need to raise errors, since it wouldn't be possible to set labelling or permissions. Of course we have to make sure we don't give bogus errors in the case of root squash NFS too. We need to consider storage pool enumeration separately from guest bootup though. Both have specific semantics and we shouldn't force them to behave the same way - the API needs to serve both their needs. Hi Daniel, About the root squash NFS issue, currently, we use access(disk_path, F_OK) to check each disk file in a diskchain. It is okay to boot up guest for disk whose backing files are located in NFS shared folder with root_squash. The disk chain will not be reported as broken in my test. It seems that qemu needs only the read permission for disk backing files and all changes on disk in furture will write to current disk file itself. So I think in this case libvirt doesn't need to do checking on NFS shared folder. Do you think it is right? Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] tests: add qemustartuppolicy testcase
--- tests/Makefile.am | 10 +- tests/qemustartuppolicytest.c | 218 + .../domain-input-cdrom-present-policy-optional.xml | 28 +++ ...domain-output-cdrom-present-policy-optional.xml | 29 +++ 4 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 tests/qemustartuppolicytest.c create mode 100644 tests/qemustartuppolicytestdata/domain-input-cdrom-present-policy-optional.xml create mode 100644 tests/qemustartuppolicytestdata/domain-output-cdrom-present-policy-optional.xml diff --git a/tests/Makefile.am b/tests/Makefile.am index a9bcf4c..e54532c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -84,6 +84,7 @@ EXTRA_DIST = \ qemuxml2argvdata \ qemuxml2xmloutdata \ qemuxmlnsdata \ + qemustartuppolicytestdata \ securityselinuxlabeldata \ schematestutils.sh \ sexpr2xmldata \ @@ -162,7 +163,7 @@ if WITH_QEMU test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \ qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \ qemumonitortest qemumonitorjsontest qemuhotplugtest \ - qemuagenttest + qemuagenttest qemustartuppolicytest endif if WITH_LXC @@ -439,12 +440,17 @@ domainsnapshotxml2xmltest_SOURCES = \ domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) + +qemustartuppolicytest_SOURCES = \ + qemustartuppolicytest.c testutilsqemu.c testutilsqemu.h \ + testutils.c testutils.h $(NULL) +qemustartuppolicytest_LDADD = $(qemu_LDADDS) else EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemuxmlnstest.c qemuhelptest.c domainsnapshotxml2xmltest.c \ qemumonitortest.c testutilsqemu.c testutilsqemu.h \ qemumonitorjsontest.c qemuhotplugtest.c \ - qemuagenttest.c \ + qemuagenttest.c qemustartuppolicytest.c \ $(QEMUMONITORTESTUTILS_SOURCES) endif diff --git a/tests/qemustartuppolicytest.c b/tests/qemustartuppolicytest.c new file mode 100644 index 000..0f0d4b8 --- /dev/null +++ b/tests/qemustartuppolicytest.c @@ -0,0 +1,218 @@ +/* + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + */ + +#include config.h + +#include stdlib.h +#include stdbool.h + +#include testutils.h + +#ifdef WITH_QEMU + +# include internal.h +# include conf/domain_event.h +# include qemu/qemu_conf.h +# include qemu/qemu_process.h +# include testutilsqemu.h + +# define VIR_FROM_THIS VIR_FROM_NONE + +static virQEMUDriver driver; + +enum { +MANDATORY, +REQUISITE, +OPTIONAL +}; + +typedef struct _qemuStartupPolicyTestData qemuStartupPolicyTestData; +struct _qemuStartupPolicyTestData { +const char *domain_filename; +int policy; +bool disk_present; +bool fail; +bool cold_boot; +}; + +static int +qemuNewDomainObjects(virDomainXMLOptionPtr xmlopt, + virDomainObjPtr *vm, + const char *filename) +{ +int ret = -1; + +if (!(*vm = virDomainObjNew(xmlopt))) +goto cleanup; + +if (!((*vm)-def = virDomainDefParseFile(filename, + driver.caps, + driver.xmlopt, + QEMU_EXPECTED_VIRT_TYPES, + 0))) +goto cleanup; + +ret = 0; + +cleanup: +return ret; +} + +static int +testQemuStartupPolicy(const void *data) +{ +int ret = -1, check_ret = -1; +qemuStartupPolicyTestData *test = (qemuStartupPolicyTestData *)data; +const char *domain_filename = test-domain_filename; +bool cold_boot = test-cold_boot; +char *inxml = NULL; +char *outxml = NULL; +char *outXmlData = NULL; +char *actual = NULL; +virDomainObjPtr vm = NULL; + +if (virAsprintf(inxml, %s/qemustartuppolicytestdata/domain-input-%s.xml, +abs_srcdir, domain_filename) 0) +goto cleanup; + +if (qemuNewDomainObjects(driver.xmlopt, vm, inxml) 0) +goto cleanup; + +check_ret = qemuDomainCheckDiskPresence(driver, vm, cold_boot); + +if (check_ret 0) { +if (test-disk_present) { +fprintf(stderr, Disk
[libvirt] [PATCH 2/4] tests: add startuppolicy testcases for cdrom
--- tests/qemustartuppolicytest.c | 4 +++ .../domain-input-cdrom-absent-policy-mandatory.xml | 28 + .../domain-input-cdrom-absent-policy-optional.xml | 28 + .../domain-input-cdrom-absent-policy-requisite.xml | 28 + .../domain-output-cdrom-absent-policy-optional.xml | 29 ++ ...domain-output-cdrom-absent-policy-requisite.xml | 29 ++ 6 files changed, 146 insertions(+) create mode 100644 tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-mandatory.xml create mode 100644 tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-optional.xml create mode 100644 tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-requisite.xml create mode 100644 tests/qemustartuppolicytestdata/domain-output-cdrom-absent-policy-optional.xml create mode 100644 tests/qemustartuppolicytestdata/domain-output-cdrom-absent-policy-requisite.xml diff --git a/tests/qemustartuppolicytest.c b/tests/qemustartuppolicytest.c index 0f0d4b8..df09507 100644 --- a/tests/qemustartuppolicytest.c +++ b/tests/qemustartuppolicytest.c @@ -196,6 +196,10 @@ mymain(void) } while (0) DO_TEST_OPTIONAL(cdrom-present-policy-optional, false); +DO_TEST_OPTIONAL(cdrom-absent-policy-optional, false); +DO_TEST_MANDATORY(cdrom-absent-policy-mandatory, false, true); +DO_TEST_REQUISITE(cdrom-absent-policy-requisite, false, true, true); +DO_TEST_REQUISITE(cdrom-absent-policy-requisite, false, false, false); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); diff --git a/tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-mandatory.xml b/tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-mandatory.xml new file mode 100644 index 000..0999aef --- /dev/null +++ b/tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-mandatory.xml @@ -0,0 +1,28 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='cdrom' + target dev='hdc' bus='ide'/ + driver name='qemu' type='raw'/ + source file='/dev/cdrom-nonexist' startupPolicy='mandatory'/ + readonly/ + address type='drive' controller='0' bus='1' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-optional.xml b/tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-optional.xml new file mode 100644 index 000..cdcc96b --- /dev/null +++ b/tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-optional.xml @@ -0,0 +1,28 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='cdrom' + target dev='hdc' bus='ide'/ + driver name='qemu' type='raw'/ + source file='/dev/cdrom-nonexist' startupPolicy='optional'/ + readonly/ + address type='drive' controller='0' bus='1' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-requisite.xml b/tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-requisite.xml new file mode 100644 index 000..8035dfb --- /dev/null +++ b/tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-requisite.xml @@ -0,0 +1,28 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='cdrom' + target dev='hdc' bus='ide'/ + driver name='qemu' type='raw'/ + source file='/dev/cdrom-nonexist' startupPolicy='requisite'/ + readonly/ +
[libvirt] tests: add qemu startupPolicy testcase
Add a group of testcase of startupPolicy features for CDROM and Harddisk. It compares the output xml string after checking the presence of disk and applying startupPolicy with expected xml string. New file: tests/qemustartuppolicytest.c TestData: tests/qemustartuppolicytestdata/ Guannan Ren(4) tests: add qemustartuppolicy testcase tests: add startuppolicy testcases for cdrom tests: add startuppolicy testcases for single harddisk tests: add startuppolicy testcase for multiple mixed harddisks tests/Makefile.am | 10 - tests/qemustartuppolicytest.c | 230 tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-mandatory.xml | 28 ++ tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-optional.xml | 28 ++ tests/qemustartuppolicytestdata/domain-input-cdrom-absent-policy-requisite.xml | 28 ++ tests/qemustartuppolicytestdata/domain-input-cdrom-present-policy-optional.xml | 28 ++ tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-mandatory.xml | 27 + tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-optional.xml | 27 + tests/qemustartuppolicytestdata/domain-input-disk-present-policy-none.xml | 27 + tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-mandatory.xml | 36 + tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-optional.xml | 36 + tests/qemustartuppolicytestdata/domain-input-multiple-disks-absent-policy-optional.xml | 36 + tests/qemustartuppolicytestdata/domain-output-cdrom-absent-policy-optional.xml | 29 ++ tests/qemustartuppolicytestdata/domain-output-cdrom-absent-policy-requisite.xml | 29 ++ tests/qemustartuppolicytestdata/domain-output-cdrom-present-policy-optional.xml | 29 ++ tests/qemustartuppolicytestdata/domain-output-disk-absent-policy-optional.xml | 22 +++ tests/qemustartuppolicytestdata/domain-output-disk-present-policy-none.xml | 28 ++ tests/qemustartuppolicytestdata/domain-output-mixed-multiple-disks-absent-policy-mandatory.xml | 36 + tests/qemustartuppolicytestdata/domain-output-mixed-multiple-disks-absent-policy-optional.xml | 27 + tests/qemustartuppolicytestdata/domain-output-multiple-disks-absent-policy-optional.xml | 22 +++ 20 files changed, 761 insertions(+), 2 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] tests: add startuppolicy testcase for multiple mixed harddisks
--- tests/qemustartuppolicytest.c | 4 +++ ...ixed-multiple-disks-absent-policy-mandatory.xml | 36 ++ ...mixed-multiple-disks-absent-policy-optional.xml | 36 ++ ...input-multiple-disks-absent-policy-optional.xml | 36 ++ ...ixed-multiple-disks-absent-policy-mandatory.xml | 36 ++ ...mixed-multiple-disks-absent-policy-optional.xml | 27 ...utput-multiple-disks-absent-policy-optional.xml | 22 + 7 files changed, 197 insertions(+) create mode 100644 tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-mandatory.xml create mode 100644 tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-optional.xml create mode 100644 tests/qemustartuppolicytestdata/domain-input-multiple-disks-absent-policy-optional.xml create mode 100644 tests/qemustartuppolicytestdata/domain-output-mixed-multiple-disks-absent-policy-mandatory.xml create mode 100644 tests/qemustartuppolicytestdata/domain-output-mixed-multiple-disks-absent-policy-optional.xml create mode 100644 tests/qemustartuppolicytestdata/domain-output-multiple-disks-absent-policy-optional.xml diff --git a/tests/qemustartuppolicytest.c b/tests/qemustartuppolicytest.c index 7081400..4757ed3 100644 --- a/tests/qemustartuppolicytest.c +++ b/tests/qemustartuppolicytest.c @@ -205,6 +205,10 @@ mymain(void) DO_TEST_OPTIONAL(disk-present-policy-none, true); DO_TEST_MANDATORY(disk-absent-policy-mandatory, false, true); +DO_TEST_OPTIONAL(multiple-disks-absent-policy-optional, false); +DO_TEST_OPTIONAL(mixed-multiple-disks-absent-policy-optional, false); +DO_TEST_MANDATORY(mixed-multiple-disks-absent-policy-mandatory, false, true); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); virObjectUnref(driver.config); diff --git a/tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-mandatory.xml b/tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-mandatory.xml new file mode 100644 index 000..b98fa80 --- /dev/null +++ b/tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-mandatory.xml @@ -0,0 +1,36 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='disk' + target dev='vda' bus='virtio'/ + driver name='qemu' type='raw'/ + source file='/dev/disk-nonexist1' startupPolicy='mandatory'/ +/disk +disk type='file' device='disk' + target dev='vdb' bus='virtio'/ + driver name='qemu' type='raw'/ + source file='/dev/null' startupPolicy='mandatory'/ +/disk +disk type='file' device='disk' + target dev='vdc' bus='virtio'/ + driver name='qemu' type='raw'/ + source file='/dev/disk-nonexist3' startupPolicy='optional'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-optional.xml b/tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-optional.xml new file mode 100644 index 000..2f68912 --- /dev/null +++ b/tests/qemustartuppolicytestdata/domain-input-mixed-multiple-disks-absent-policy-optional.xml @@ -0,0 +1,36 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='disk' + target dev='vda' bus='virtio'/ + driver name='qemu' type='raw'/ + source file='/dev/disk-nonexist1' startupPolicy='optional'/ +/disk +disk type='file' device='disk' + target dev='vdb' bus='virtio'/ + driver name='qemu' type='raw'/ + source file='/dev/null' startupPolicy='mandatory'/ +/disk +disk type='file' device='disk' + target dev='vdc' bus='virtio'/ + driver name='qemu' type='raw'/ + source file='/dev/disk-nonexist3' startupPolicy='optional'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git
[libvirt] [PATCH 3/4] tests: add startuppolicy testcases for single harddisk
--- tests/qemustartuppolicytest.c | 4 .../domain-input-disk-absent-policy-mandatory.xml | 27 + .../domain-input-disk-absent-policy-optional.xml | 27 + .../domain-input-disk-present-policy-none.xml | 27 + .../domain-output-disk-absent-policy-optional.xml | 22 + .../domain-output-disk-present-policy-none.xml | 28 ++ 6 files changed, 135 insertions(+) create mode 100644 tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-mandatory.xml create mode 100644 tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-optional.xml create mode 100644 tests/qemustartuppolicytestdata/domain-input-disk-present-policy-none.xml create mode 100644 tests/qemustartuppolicytestdata/domain-output-disk-absent-policy-optional.xml create mode 100644 tests/qemustartuppolicytestdata/domain-output-disk-present-policy-none.xml diff --git a/tests/qemustartuppolicytest.c b/tests/qemustartuppolicytest.c index df09507..7081400 100644 --- a/tests/qemustartuppolicytest.c +++ b/tests/qemustartuppolicytest.c @@ -201,6 +201,10 @@ mymain(void) DO_TEST_REQUISITE(cdrom-absent-policy-requisite, false, true, true); DO_TEST_REQUISITE(cdrom-absent-policy-requisite, false, false, false); +DO_TEST_OPTIONAL(disk-absent-policy-optional, false); +DO_TEST_OPTIONAL(disk-present-policy-none, true); +DO_TEST_MANDATORY(disk-absent-policy-mandatory, false, true); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); virObjectUnref(driver.config); diff --git a/tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-mandatory.xml b/tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-mandatory.xml new file mode 100644 index 000..5164a4e --- /dev/null +++ b/tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-mandatory.xml @@ -0,0 +1,27 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='disk' + target dev='hdc' bus='ide'/ + driver name='qemu' type='raw'/ + source file='/dev/disk-nonexist' startupPolicy='mandatory'/ + address type='drive' controller='0' bus='1' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-optional.xml b/tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-optional.xml new file mode 100644 index 000..8d25ea9 --- /dev/null +++ b/tests/qemustartuppolicytestdata/domain-input-disk-absent-policy-optional.xml @@ -0,0 +1,27 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='disk' + target dev='hdc' bus='ide'/ + driver name='qemu' type='raw'/ + source file='/dev/disk-nonexist' startupPolicy='optional'/ + address type='drive' controller='0' bus='1' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemustartuppolicytestdata/domain-input-disk-present-policy-none.xml b/tests/qemustartuppolicytestdata/domain-input-disk-present-policy-none.xml new file mode 100644 index 000..1ec6a90 --- /dev/null +++ b/tests/qemustartuppolicytestdata/domain-input-disk-present-policy-none.xml @@ -0,0 +1,27 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='disk' + target dev='hdc' bus='ide'/ + driver name='qemu' type='raw'/ + source file='/dev/null'/ + address type='drive' controller='0' bus='1' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +
Re: [libvirt] [test-API][PATCH] Fix the repeating display of testcase name issue when generating log.xml
On 08/16/2013 02:36 PM, Hongming Zhang wrote: modified: src/log_generator.py --- src/log_generator.py |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/log_generator.py b/src/log_generator.py index de18654..7685da7 100644 --- a/src/log_generator.py +++ b/src/log_generator.py @@ -70,7 +70,8 @@ class LogGenerator(object): valuedict = test_procedure[casename] test_casename = self.doc.createElement('action') -test_casename.setAttribute('name', casename) +casenamexml = casename[:casename.rfind(:)] +test_casename.setAttribute('name', casenamexml) for arg in valuedict.keys(): test_arg = self.doc.createElement('arg') Hi HongMing you fixed the problem in not right way, you need to change the casename in the source rather than do the string slice in there. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] Add display of cases result to log.xml
On 08/15/2013 02:00 PM, Alex Jia wrote: Hi hongming, BTW, could we ignore module or function name in Test Procedure section, because the module and entry function are the same name, it's a deliberately design, but the Test Procedure looks like a duplicate naming for others. I agree with you. There are three pre-defined functions for each $testcase.py $testcase_check (optional) which is used to check if the testcase is runnable. $testcase which is the testing body for actual test work. $testcase_clean (optional) is used to clean testing environment after test. we can only list the names of $testcase_check and $testcase_clean in the third column and ignore the $testcase, do you think it is good idea? slice network:define:define network:network_list:network_list network:start:start network:network_list:network_list network:autostart:autostart network:update:update /slice -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] Add display of cases result to log.xml
On 08/15/2013 02:31 PM, Alex Jia wrote: Gren, I think it should be enough if we can know which test cases is run, and should hide some details such as checkpoint and clean function. Okay, anyway, we have detailed log to check. without the function name, the UI could become more concise. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] Add display of cases result to log.xml
On 08/15/2013 01:47 PM, hongming wrote: Hi Guannan You can see the new test report from the following url. http://fileshare.englab.nay.redhat.com/pub/section3/libvirtauto/libvirt-test-API/log.xml Thanks Hongming Hi hongming, Patch looks good, ACK. Next time, please don't send non-public website to upstream which will confuse others. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] Add network update test case
On 08/09/2013 05:28 PM, hongming zhang wrote: The patch add network update test case to cover network update api. modified: cases/basic_network.conf - The test suite covers add/modify/delete ip-dhcp-host in network. For other test scenario, they can be test via customing the conf file. new file: repos/network/update.py - So far the network update test case only checking when flag is live or current. new file: repos/network/xmls/ip-dhcp-host.xml - Create the ip-dhcp-host sample xml file for adding/deleting ip-dhcp-host new file: repos/network/xmls/modify-ip-dhcp-host.xml - Create the ip-dhcp-host sample xml file for modifying ip-dhcp-host Known bug: - Bug 985782 - Some flag values of method are missing in libvirt-python bindings --- cases/basic_network.conf | 28 ++ repos/network/update.py| 82 repos/network/xmls/ip-dhcp-host.xml|1 + repos/network/xmls/modify-ip-dhcp-host.xml |1 + 4 files changed, 112 insertions(+), 0 deletions(-) create mode 100644 repos/network/update.py create mode 100644 repos/network/xmls/ip-dhcp-host.xml create mode 100644 repos/network/xmls/modify-ip-dhcp-host.xml diff --git a/cases/basic_network.conf b/cases/basic_network.conf index 91d7f21..e9abd57 100644 --- a/cases/basic_network.conf +++ b/cases/basic_network.conf @@ -32,6 +32,34 @@ network:autostart autostart enable +network:update +networkname + $defaultnetname +command +add-first +section +ip-dhcp-host + +network:update +networkname + $defaultnetname +command +modify +section +ip-dhcp-host +xml + xmls/modify-ip-dhcp-host.xml + +network:update +networkname + $defaultnetname +command +delete +section +ip-dhcp-host +xml + xmls/modify-ip-dhcp-host.xml + network:destroy networkname $defaultnetname diff --git a/repos/network/update.py b/repos/network/update.py new file mode 100644 index 000..0024a5e --- /dev/null +++ b/repos/network/update.py @@ -0,0 +1,82 @@ +#!/usr/bin/evn python +#Update a network + +import libvirt +from libvirt import libvirtError +from src import sharedmod + +COMMANDDICT = {none:0, modify:1, delete:2, add-first:4} +SECTIONDICT = {none:0, bridge:1, domain:2, ip:3, ip-dhcp-host:4, \ + ip-dhcp-range:5, forward:6, forward-interface:7,\ + forward-pf:8, portgroup:9, dns-host:10, dns-txt:11,\ + dns-srv:12} +FLAGSDICT = {current:0, live:1, config: 2} + +required_params = ('networkname', ) +optional_params = { + 'command': 'add-first', + 'section': 'ip-dhcp-host', + 'parentIndex': 0, + 'xml': 'xmls/ip-dhcp-host.xml', + 'flag': 'current', + } + +def update(params): +Update a network from xml +global logger +logger = params['logger'] +networkname = params['networkname'] +conn = sharedmod.libvirtobj['conn'] + +command = params['command'] +logger.info(The specified command is %s % command) +section = params['section'] +logger.info(The specified section is %s % section) +parentIndex = int(params.get('parentIndex', 0)) +logger.info(The specified parentIndex is %d % parentIndex) +xmlstr = params.get('xml', 'xmls/ip-dhcp-host.xml').replace('\','\'') +logger.info(The specified updatexml is %s % xmlstr) +flag = params.get('flag', 'current') +logger.info(The specified flag is %s % flag) + +command_val = 0 +section_val = 0 +flag_val = 0 +if COMMANDDICT.has_key(command): +command_val = COMMANDDICT.get(command) +if SECTIONDICT.has_key(section): +section_val = SECTIONDICT.get(section) +if FLAGSDICT.has_key(flag): +flag_val = FLAGSDICT.get(flag) + +try: +network = conn.networkLookupByName(networkname) +logger.info(The original network xml is %s % network.XMLDesc(0)) +network.update(command_val, section_val, parentIndex, xmlstr, \ + flag_val) +updated_netxml = network.XMLDesc(0) +logger.info(The updated network xml is %s % updated_netxml) +#The check only works when flag isn't set as config +if flag_val !=2: +if command_val == 0 or command_val == 2: +if xmlstr not in updated_netxml: +logger.info(Successfully update network) +return 0 +else: +logger.error(Failed to update network) +return 1 + +elif command_val == 1 or command_val == 4: +if xmlstr in updated_netxml: +logger.info(Successfully update network) +return 0 +else: +logger.error(Failed to update network) +return 1 + +except
Re: [libvirt] [PATCH v6 2/2] qemu: support to drop disk with 'optional' startupPolicy
On 08/06/2013 09:40 PM, Martin Kletzander wrote: On 08/02/2013 08:37 AM, Guannan Ren wrote: Go through disks of guest, if one disk doesn't exist or its backing chain is broken, with 'optional' startupPolicy, for CDROM and Floppy we only discard its source path definition in xml, for disks we drop it from disk list and free it. --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_domain.c | 69 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7bd3559..52ac95d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4727,6 +4727,7 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn, */ typedef enum { VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */ +VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START = 1, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_DISK_CHANGE_LAST diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1ff802c..6794e26 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c [...] @@ -2080,11 +2111,12 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, bool cold_boot) { int ret = -1; -size_t i; +ssize_t i; virDomainDiskDefPtr disk; +size_t ndisks = vm-def-ndisks; VIR_DEBUG(Checking for disk presence); -for (i = 0; i vm-def-ndisks; i++) { +for (i = ndisks - 1; i = 0; i--) { disk = vm-def-disks[i]; Now I noticed that there should be either (1) a check for domain without any disks (if ndisks == 0; return 0) or (2) a different for loop, like this for example: for (i = vm-def-ndisks; i 0; i--) { disk = vm-def-disks[i - 1]; I know that I proposed the faulty version, sorry for that. From those 2 versions I like the second one better because it looks cleaner and you can skip first lines of this hunk. It'd be nice to have a test for this to make sure we won't break it in future. At least xml2xml test if we can't test the disk dropping now. ACK with that fixed and this patch squashed in the previous one (also with those mentioned things fixed). Martin pushed with these mentioned fixed. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 2/2] qemu: support to drop disk with 'optional' startupPolicy
On 08/06/2013 09:40 PM, Martin Kletzander wrote: On 08/02/2013 08:37 AM, Guannan Ren wrote: Go through disks of guest, if one disk doesn't exist or its backing chain is broken, with 'optional' startupPolicy, for CDROM and Floppy we only discard its source path definition in xml, for disks we drop it from disk list and free it. --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_domain.c | 69 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7bd3559..52ac95d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4727,6 +4727,7 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn, */ typedef enum { VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */ +VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START = 1, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_DISK_CHANGE_LAST diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1ff802c..6794e26 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c [...] @@ -2080,11 +2111,12 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, bool cold_boot) { int ret = -1; -size_t i; +ssize_t i; virDomainDiskDefPtr disk; +size_t ndisks = vm-def-ndisks; VIR_DEBUG(Checking for disk presence); -for (i = 0; i vm-def-ndisks; i++) { +for (i = ndisks - 1; i = 0; i--) { disk = vm-def-disks[i]; Now I noticed that there should be either (1) a check for domain without any disks (if ndisks == 0; return 0) or (2) a different for loop, like this for example: for (i = vm-def-ndisks; i 0; i--) { disk = vm-def-disks[i - 1]; I know that I proposed the faulty version, sorry for that. From those 2 versions I like the second one better because it looks cleaner and you can skip first lines of this hunk. It's good you noticed it in time. my local testcase doesn't include the case of domain without disks. I should add it. It'd be nice to have a test for this to make sure we won't break it in future. At least xml2xml test if we can't test the disk dropping now. Make sense. ACK with that fixed and this patch squashed in the previous one (also with those mentioned things fixed). Martin Thanks for this review. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 2/2] qemu: support to drop disk with 'optional' startupPolicy
Go through disks of guest, if one disk doesn't exist or its backing chain is broken, with 'optional' startupPolicy, for CDROM and Floppy we only discard its source path definition in xml, for disks we drop it from disk list and free it. --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_domain.c | 69 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7bd3559..52ac95d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4727,6 +4727,7 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn, */ typedef enum { VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */ +VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START = 1, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_DISK_CHANGE_LAST diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1ff802c..6794e26 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2027,13 +2027,53 @@ cleanup: } static int +qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ +char uuid[VIR_UUID_STRING_BUFLEN]; +virDomainEventPtr event = NULL; +virDomainDiskDefPtr del_disk = NULL; + +virUUIDFormat(vm-def-uuid, uuid); + +VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') + due to inaccessible source '%s', + disk-dst, vm-def-name, uuid, disk-src); + +if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM || +disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { + +event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, + disk-info.alias, + VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); +VIR_FREE(disk-src); +} else { +event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, + disk-info.alias, + VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START); + +if (!(del_disk = virDomainDiskRemoveByName(vm-def, disk-src))) { +virReportError(VIR_ERR_INVALID_ARG, + _(no source device %s), disk-src); +return -1; +} +virDomainDiskDefFree(del_disk); +} + +if (event) +qemuDomainEventQueue(driver, event); + +return 0; +} + +static int qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, bool cold_boot) { char uuid[VIR_UUID_STRING_BUFLEN]; -virDomainEventPtr event = NULL; int startupPolicy = disk-startupPolicy; virUUIDFormat(vm-def-uuid, uuid); @@ -2056,17 +2096,8 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; } -virResetLastError(); -VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') - due to inaccessible source '%s', - disk-dst, vm-def-name, uuid, disk-src); - -event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, disk-info.alias, - VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); -if (event) -qemuDomainEventQueue(driver, event); - -VIR_FREE(disk-src); +if (qemuDomainCheckRemoveOptionalDisk(driver, vm, disk) 0) +goto error; return 0; @@ -2080,11 +2111,12 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, bool cold_boot) { int ret = -1; -size_t i; +ssize_t i; virDomainDiskDefPtr disk; +size_t ndisks = vm-def-ndisks; VIR_DEBUG(Checking for disk presence); -for (i = 0; i vm-def-ndisks; i++) { +for (i = ndisks - 1; i = 0; i--) { disk = vm-def-disks[i]; if (!disk-src) @@ -2094,10 +2126,11 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, qemuDiskChainCheckBroken(disk) = 0) continue; -if (disk-startupPolicy) { -if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, - cold_boot) = 0) -continue; +if (disk-startupPolicy +qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + cold_boot) = 0) { +virResetLastError(); +continue; } goto error; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 0/2] add startupPolicy support for harddisks
These patches are based on code https://www.redhat.com/archives/libvir-list/2013-July/msg01741.html The set of patches is trying to add 'startupPolicy' support for guest hard disks. For the 'optional' policy, there is a little difference from CDROM and Floppy which only drop its source path, for disks, if missing, the checking function will drop their definitions, because qemu doesn't allow missing source path for hard disk. migration is not supported yet. Guannan Ren(2) [PATCH v5 1/2] conf: add startupPolicy attribute for harddisk [PATCH v5 2/2] qemu: support to drop disk with 'optional' startupPolicy docs/formatdomain.html.in | 8 ++-- docs/schemas/domaincommon.rng | 6 ++ include/libvirt/libvirt.h.in | 1 + src/conf/domain_conf.c| 31 +++ src/qemu/qemu_domain.c| 77 + 5 files changed, 97 insertions(+), 26 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 1/2] conf: add startupPolicy attribute for harddisk
Add startupPolicy attribute policy for harddisk with type file, block and dir. 'requisite' is not supported currently for hardisk. --- docs/formatdomain.html.in | 8 ++-- docs/schemas/domaincommon.rng | 6 ++ src/conf/domain_conf.c| 31 +++ 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 78e132e..cbfc773 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1620,7 +1620,8 @@ policy what to do with the disk if the source file is not accessible. (NB, codestartupPolicy/code is not valid for volume disk unless the specified storage volume is of file type). This is done by the -codestartupPolicy/code attribute, accepting these values: +codestartupPolicy/code attribute(span class=sinceSince 0.9.7/span), +accepting these values: table class=top_table tr td mandatory /td @@ -1636,7 +1637,10 @@ td drop if missing at any start attempt /td /tr /table -span class=sinceSince 0.9.7/span +span class=sinceSince 1.1.2/span the codestartupPolicy/code is extended +to support hard disks besides cdrom and floppy. On guest cold bootup, if a certain disk +is not accessible or its disk chain is broken, with startupPolicy 'optional' the guest +will drop this disk. This feature doesn't support migration currently. /dd dtcodemirror/code/dt dd diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 745b959..c6ee01c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1124,6 +1124,9 @@ ref name=absFilePath/ /attribute optional + ref name=startupPolicy/ +/optional +optional ref name='devSeclabel'/ /optional /element @@ -1141,6 +1144,9 @@ attribute name=dir ref name=absFilePath/ /attribute +optional + ref name=startupPolicy/ +/optional empty/ /element /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a86be8c..e764492 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4795,7 +4795,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, switch (def-type) { case VIR_DOMAIN_DISK_TYPE_FILE: source = virXMLPropString(cur, file); -startupPolicy = virXMLPropString(cur, startupPolicy); break; case VIR_DOMAIN_DISK_TYPE_BLOCK: source = virXMLPropString(cur, dev); @@ -4882,7 +4881,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, case VIR_DOMAIN_DISK_TYPE_VOLUME: if (virDomainDiskSourcePoolDefParse(cur, def) 0) goto error; -startupPolicy = virXMLPropString(cur, startupPolicy); break; default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -4891,6 +4889,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } +startupPolicy = virXMLPropString(cur, startupPolicy); + /* People sometimes pass a bogus '' source path when they mean to omit the source element completely (e.g. CDROM without media). This is @@ -5463,14 +5463,22 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } -if (def-device != VIR_DOMAIN_DISK_DEVICE_CDROM -def-device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { -virReportError(VIR_ERR_INVALID_ARG, - _(Setting disk %s is allowed only for - cdrom or floppy), +if (def-type == VIR_DOMAIN_DISK_TYPE_NETWORK) { +virReportError(VIR_ERR_XML_ERROR, + _(Setting disk %s is not allowed for + disk of network type), startupPolicy); goto error; } + +if (def-device != VIR_DOMAIN_DISK_DEVICE_CDROM +def-device != VIR_DOMAIN_DISK_DEVICE_FLOPPY +val == VIR_DOMAIN_STARTUP_POLICY_REQUISITE) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Setting disk 'requisite' only for + cdrom or floppy)); +goto error; +} def-startupPolicy = val; } @@ -14125,6 +14133,9 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, case VIR_DOMAIN_DISK_TYPE_BLOCK:
[libvirt] [PATCH v5 2/2] qemu: support to drop disk with 'optional' startupPolicy
Go through disks of guest, if one disk doesn't exist or its backing chain is broken, with 'optional' startupPolicy, for CDROM and Floppy we only discard its source path definition in xml, for disks we drop it from disk list and free it. --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_domain.c | 77 +++- 2 files changed, 62 insertions(+), 16 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c0eb25b..3888ba5 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4728,6 +4728,7 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn, */ typedef enum { VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */ +VIR_DOMAIN_EVENT_HARDDISK_DROP_MISSING_ON_START = 1, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_DISK_CHANGE_LAST diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1ff802c..5da6e18 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2027,13 +2027,59 @@ cleanup: } static int +qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + size_t *nextdisk) +{ +char uuid[VIR_UUID_STRING_BUFLEN]; +virDomainEventPtr event = NULL; +virDomainDiskDefPtr del_disk = NULL; + +virUUIDFormat(vm-def-uuid, uuid); + +VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') + due to inaccessible source '%s', + disk-dst, vm-def-name, uuid, disk-src); + +if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM || +disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { + +event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, + disk-info.alias, + VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); + /* For cdrom or floppy, we only remove its source definition +* So the nextdisk need to point to next disk. +*/ +(*nextdisk)++; +VIR_FREE(disk-src); +} else { +event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, + disk-info.alias, + VIR_DOMAIN_EVENT_HARDDISK_DROP_MISSING_ON_START); + +if (!(del_disk = virDomainDiskRemoveByName(vm-def, disk-src))) { +virReportError(VIR_ERR_INVALID_ARG, + _(no source device %s), disk-src); +return -1; +} +virDomainDiskDefFree(del_disk); +} + +if (event) +qemuDomainEventQueue(driver, event); + +return 0; +} + +static int qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, - bool cold_boot) + bool cold_boot, + size_t *nextdisk) { char uuid[VIR_UUID_STRING_BUFLEN]; -virDomainEventPtr event = NULL; int startupPolicy = disk-startupPolicy; virUUIDFormat(vm-def-uuid, uuid); @@ -2057,16 +2103,8 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, } virResetLastError(); -VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') - due to inaccessible source '%s', - disk-dst, vm-def-name, uuid, disk-src); - -event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, disk-info.alias, - VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); -if (event) -qemuDomainEventQueue(driver, event); - -VIR_FREE(disk-src); +if (qemuDomainCheckRemoveOptionalDisk(driver, vm, disk, nextdisk) 0) +goto error; return 0; @@ -2082,21 +2120,28 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, int ret = -1; size_t i; virDomainDiskDefPtr disk; +size_t count = vm-def-ndisks; +size_t nextdisk = 0; + VIR_DEBUG(Checking for disk presence); -for (i = 0; i vm-def-ndisks; i++) { -disk = vm-def-disks[i]; +for (i = 0; i count; i++) { +disk = vm-def-disks[nextdisk]; + if (!disk-src) continue; if (qemuDomainDetermineDiskChain(driver, disk, false) = 0 -qemuDiskChainCheckBroken(disk) = 0) +qemuDiskChainCheckBroken(disk) = 0) { +nextdisk++; continue; +} if (disk-startupPolicy) { if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, - cold_boot) = 0) + cold_boot, + nextdisk) = 0)
[libvirt] [PATCH v4 3/5] qemu: check presence of each disk and its backing file as well
v3-v4 fix code indentation issue. For disk with startupPolicy support, such as cdrom and floppy when its chain is broken, the startup policy will apply, otherwise, report an error. --- src/qemu/qemu_domain.c | 33 + src/qemu/qemu_process.c | 6 -- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index be77991..1ff802c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2043,19 +2043,11 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); goto error; -break; case VIR_DOMAIN_STARTUP_POLICY_REQUISITE: -if (cold_boot) { -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); +if (cold_boot) goto error; -} break; case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: @@ -2064,6 +2056,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; } +virResetLastError(); VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') due to inaccessible source '%s', disk-dst, vm-def-name, uuid, disk-src); @@ -2089,30 +2082,30 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, int ret = -1; size_t i; virDomainDiskDefPtr disk; -virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +VIR_DEBUG(Checking for disk presence); for (i = 0; i vm-def-ndisks; i++) { disk = vm-def-disks[i]; -if (!disk-startupPolicy || !disk-src) +if (!disk-src) continue; -if (virFileAccessibleAs(disk-src, F_OK, -cfg-user, -cfg-group) = 0) { -/* disk accessible */ +if (qemuDomainDetermineDiskChain(driver, disk, false) = 0 +qemuDiskChainCheckBroken(disk) = 0) continue; + +if (disk-startupPolicy) { +if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + cold_boot) = 0) +continue; } -if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, - cold_boot) 0) -goto cleanup; +goto error; } ret = 0; -cleanup: -virObjectUnref(cfg); +error: return ret; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e8e459e..61a897c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3621,16 +3621,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuAssignDeviceAliases(vm-def, priv-qemuCaps) 0) goto cleanup; -VIR_DEBUG(Checking for CDROM and floppy presence); if (qemuDomainCheckDiskPresence(driver, vm, flags VIR_QEMU_PROCESS_START_COLD) 0) goto cleanup; -for (i = 0; i vm-def-ndisks; i++) { -if (qemuDomainDetermineDiskChain(driver, vm-def-disks[i], - false) 0) -goto cleanup; -} /* Get the advisory nodeset from numad if 'placement' of * either vcpu or numatune is 'auto'. -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/3] qemu: check presence of each disk and its backing file as well
On 07/31/2013 04:49 PM, Martin Kletzander wrote: On 07/30/2013 08:26 AM, Guannan Ren wrote: For disk with startupPolicy support, such as cdrom and floppy when its chain is broken, the startup policy will apply, otherwise, report an error. --- src/qemu/qemu_domain.c | 31 +-- src/qemu/qemu_process.c | 6 -- 2 files changed, 13 insertions(+), 24 deletions(-) And rewrite this to one condition. So basically ACK with this squashed in: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1e75adb..c54f9f6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2056,7 +2056,6 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; } -virResetLastError(); VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') due to inaccessible source '%s', disk-dst, vm-def-name, uuid, disk-src); @@ -2095,10 +2094,11 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, qemuDiskChainCheckBroken(disk) = 0) continue; -if (disk-startupPolicy) { -if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, - cold_boot) = 0) -continue; +if (disk-startupPolicy +qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + cold_boot) = 0) { +virResetLastError(); +continue; } goto cleanup; -- Martin Thanks for the review. pushed. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/3] qemu: check presence of each disk and its backing file as well
On 07/31/2013 04:49 PM, Martin Kletzander wrote: On 07/30/2013 08:26 AM, Guannan Ren wrote: For disk with startupPolicy support, such as cdrom and floppy when its chain is broken, the startup policy will apply, otherwise, report an error. --- src/qemu/qemu_domain.c | 31 +-- src/qemu/qemu_process.c | 6 -- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index be77991..1e75adb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2043,19 +2043,11 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); goto error; -break; case VIR_DOMAIN_STARTUP_POLICY_REQUISITE: -if (cold_boot) { -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); +if (cold_boot) goto error; -} break; case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: @@ -2064,6 +2056,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; } +virResetLastError(); Even though it makes perfect sense to have it here, I'd move it one function up, because it seems more readable to me. VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') due to inaccessible source '%s', disk-dst, vm-def-name, uuid, disk-src); @@ -2091,22 +2084,24 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virDomainDiskDefPtr disk; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +VIR_DEBUG(Checking for disk presence); for (i = 0; i vm-def-ndisks; i++) { disk = vm-def-disks[i]; -if (!disk-startupPolicy || !disk-src) +if (!disk-src) continue; -if (virFileAccessibleAs(disk-src, F_OK, -cfg-user, -cfg-group) = 0) { -/* disk accessible */ -continue; +if (qemuDomainDetermineDiskChain(driver, disk, false) = 0 +qemuDiskChainCheckBroken(disk) = 0) +continue; + +if (disk-startupPolicy) { +if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + cold_boot) = 0) And rewrite this to one condition. It's okay to rewrite it to one condition, but existing format is more readable. The outer if clause is used to check whether disk have startupPolicy set. The inter if clause is used to do the actual startupPolicy work. Maybe later, we can do more work for disk with startupPolicy attribute in its xml definition. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/3] qemu: check presence of each disk and its backing file as well
For disk with startupPolicy support, such as cdrom and floppy when its chain is broken, the startup policy will apply, otherwise, report an error. --- src/qemu/qemu_domain.c | 31 +-- src/qemu/qemu_process.c | 6 -- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index be77991..1e75adb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2043,19 +2043,11 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); goto error; -break; case VIR_DOMAIN_STARTUP_POLICY_REQUISITE: -if (cold_boot) { -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); +if (cold_boot) goto error; -} break; case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: @@ -2064,6 +2056,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; } +virResetLastError(); VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') due to inaccessible source '%s', disk-dst, vm-def-name, uuid, disk-src); @@ -2091,22 +2084,24 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virDomainDiskDefPtr disk; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +VIR_DEBUG(Checking for disk presence); for (i = 0; i vm-def-ndisks; i++) { disk = vm-def-disks[i]; -if (!disk-startupPolicy || !disk-src) +if (!disk-src) continue; -if (virFileAccessibleAs(disk-src, F_OK, -cfg-user, -cfg-group) = 0) { -/* disk accessible */ -continue; +if (qemuDomainDetermineDiskChain(driver, disk, false) = 0 +qemuDiskChainCheckBroken(disk) = 0) +continue; + +if (disk-startupPolicy) { +if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + cold_boot) = 0) +continue; } -if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, - cold_boot) 0) -goto cleanup; +goto cleanup; } ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e8e459e..61a897c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3621,16 +3621,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuAssignDeviceAliases(vm-def, priv-qemuCaps) 0) goto cleanup; -VIR_DEBUG(Checking for CDROM and floppy presence); if (qemuDomainCheckDiskPresence(driver, vm, flags VIR_QEMU_PROCESS_START_COLD) 0) goto cleanup; -for (i = 0; i vm-def-ndisks; i++) { -if (qemuDomainDetermineDiskChain(driver, vm-def-disks[i], - false) 0) -goto cleanup; -} /* Get the advisory nodeset from numad if 'placement' of * either vcpu or numatune is 'auto'. -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/3] qemu: add helper functions for diskchain checking
On 07/29/2013 07:27 PM, Martin Kletzander wrote: On 07/26/2013 02:37 PM, Guannan Ren wrote: *src/util/virstoragefile.c: Add a helper function to get the first name of missing backing files, if the name is NULL, it means the diskchain is not broken. *src/qemu/qemu_domain.c: qemuDiskChainCheckBroken(disk) to check if its chain is broken --- src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c| 22 ++ src/qemu/qemu_domain.h| 3 +++ src/util/virstoragefile.c | 47 +++ src/util/virstoragefile.h | 2 ++ 5 files changed, 75 insertions(+) [...] diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cb6df91..e02e28a 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -572,6 +572,13 @@ virFindBackingFile(const char *start, bool start_is_dir, const char *path, goto cleanup; } +if (virFileAccessibleAs(combined, F_OK, getuid(), getgid()) 0) { +virReportSystemError(errno, + _(Cannot access backing file '%s'), + combined); +goto cleanup; +} + As mentioned before, the pre-existing problem here is that the disk chain will be reported as broken in case root doesn't have access to the folder the file is in (for example). Could we somehow pass a function or seclabel to fallback to (from the driver)? Just a hypothetical question, no need to fix that in this series. yeah, right, I will try to resolve it in separate patch. if (!(*canonical = canonicalize_file_name(combined))) { virReportSystemError(errno, _(Can't canonicalize path '%s'), path); @@ -1097,6 +1104,46 @@ virStorageFileGetMetadata(const char *path, int format, } /** + * virStorageFileChainCheckBroken + * + * Check whether CHAIN is broken, return the broken file name if yes, + * otherwise return NULL + */ This doesn't cope with the functionality, probably missed it when reworking the function. +int +virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, + char **brokenFile) +{ +virStorageFileMetadataPtr tmp; +char *file = NULL; no need to have this variable, you can use bokenFile all the way (probably leftover before some rework). ACK with those two things fixed. Both this and the previous (I forgot to mention it there) should go in after 1.1.1 release. Even though it fixes an error, it mainly prepares the field for other patches and I'd rather wait with this. Don't hesitate to disagree though, we can discuss that on IRC with other in case there's huge need for that. Martin Thanks for the review. I will send a v3 version with these fixed -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/3] qemu: add helper functions for diskchain checking
*src/util/virstoragefile.c: Add a helper function to get the first name of missing backing files, if the name is NULL, it means the diskchain is not broken. *src/qemu/qemu_domain.c: qemuDiskChainCheckBroken(disk) to check if its chain is broken --- src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c| 22 ++ src/qemu/qemu_domain.h| 3 +++ src/util/virstoragefile.c | 46 ++ src/util/virstoragefile.h | 2 ++ 5 files changed, 74 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d9615ea..f7166d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1882,6 +1882,7 @@ virSocketAddrSetPort; # util/virstoragefile.h +virStorageFileChainGetBroken; virStorageFileChainLookup; virStorageFileFeatureTypeFromString; virStorageFileFeatureTypeToString; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 03a2aa6..be77991 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2187,6 +2187,28 @@ qemuDomainCleanupRun(virQEMUDriverPtr driver, } int +qemuDiskChainCheckBroken(virDomainDiskDefPtr disk) +{ +char *brokenFile = NULL; + +if (!disk-src || !disk-backingChain) +return 0; + +if (virStorageFileChainGetBroken(disk-backingChain, brokenFile) 0) +return -1; + +if (brokenFile) { +virReportError(VIR_ERR_INVALID_ARG, + _(Backing file '%s' of image '%s' is missing.), + brokenFile, disk-src); +VIR_FREE(brokenFile); +return -1; +} + +return 0; +} + +int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, bool force) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9a959d6..0a4a51e 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -347,6 +347,9 @@ bool qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv, int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virDomainObjPtr vm, bool start_with_state); + +int qemuDiskChainCheckBroken(virDomainDiskDefPtr disk); + int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, bool force); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cb6df91..0b9cec3 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -572,6 +572,13 @@ virFindBackingFile(const char *start, bool start_is_dir, const char *path, goto cleanup; } +if (virFileAccessibleAs(combined, F_OK, getuid(), getgid()) 0) { +virReportSystemError(errno, + _(Cannot access backing file '%s'), + combined); +goto cleanup; +} + if (!(*canonical = canonicalize_file_name(combined))) { virReportSystemError(errno, _(Can't canonicalize path '%s'), path); @@ -1097,6 +1104,45 @@ virStorageFileGetMetadata(const char *path, int format, } /** + * virStorageFileChainCheckBroken + * + * If CHAIN is broken, set *brokenFile to the broken file name, + * otherwise set it to NULL. Caller MUST free *brokenFile after use. + * Return 0 on success, negative on error. + */ +int +virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, + char **brokenFile) +{ +virStorageFileMetadataPtr tmp; +int ret = -1; + +if (!chain) +return 0; + +*brokenFile = NULL; + +tmp = chain; +while (tmp) { +/* Break if no backing store or backing store is not file */ + if (!tmp-backingStoreRaw) + break; + if (!tmp-backingStore) { + if (VIR_STRDUP(*brokenFile, tmp-backingStoreRaw) 0) + goto error; + break; + } + tmp = tmp-backingMeta; +} + +ret = 0; + +error: +return ret; +} + + +/** * virStorageFileFreeMetadata: * * Free pointers in passed structure and structure itself. diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 4cb47e6..1f89839 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -90,6 +90,8 @@ virStorageFileMetadataPtr virStorageFileGetMetadata(const char *path, virStorageFileMetadataPtr virStorageFileGetMetadataFromFD(const char *path, int fd, int format); +int virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, + char **broken_file); const char *virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start, -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com
Re: [libvirt] [v2 PATCH] caps: use -device for primary video when qemu =1.6
On 07/27/2013 12:11 AM, Doug Goldstein wrote: On Fri, Jul 26, 2013 at 10:17 AM, Eric Blake ebl...@redhat.com wrote: On 07/26/2013 06:53 AM, Guannan Ren wrote: https://bugzilla.redhat.com/show_bug.cgi?id=981094 The commit 0ad9025ef introduce qemu flag QEMU_CAPS_DEVICE_VIDEO_PRIMARY for using -device VGA, -device cirrus-vga, -device vmware-svga and -device qxl-vga. In use, for -device qxl-vga, mouse doesn't display in guest window like the desciption in above bug. This patch try to use -device for primary video when qemu =1.6 which contains the bug fix patch --- src/qemu/qemu_capabilities.c | 6 +++--- tests/qemuhelptest.c | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) ACK. Thanks, pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org Please push this into v1.1.0-maint as well. Pushed it. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/3] ensure diskchain is not broken on guest bootup
v1-v2: Add a helper function qemuDiskChainCheckBroken to check diskchain after invoking qemuDomainDetermineDiskChain() This patchset try to check whether the diskchain is broken or not when domain boot up. If dischain is broken, report like: virsh start rhel6qcow2 error: Failed to start domain rhel6qcow2 error: invalid argument: Backing file '/var/lib/libvirt/images/thirddisk' of \ image '/var/lib/libvirt/images/thirddisk.snap3' is missing. For storage pool, it still can list volumes out even if their diskchain is broken Guannan Ren(3) qemu: refactor qemuDomainCheckDiskPresence for only disk presence check qemu: add helper functions for diskchain checking qemu: check presence of each disk and its backing file as well src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c| 133 +- src/qemu/qemu_domain.h| 3 ++ src/qemu/qemu_process.c | 6 src/util/virstoragefile.c | 47 +++ src/util/virstoragefile.h | 2 ++ 6 files changed, 141 insertions(+), 51 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] qemu: refactor qemuDomainCheckDiskPresence for only disk presence check
Refactor this function to make it focus on disk presence checking, including diskchain checking, and not only for CDROM and Floppy. This change is good for the following patches. --- src/qemu/qemu_domain.c | 98 +- 1 file changed, 58 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da3b768..03a2aa6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2026,6 +2026,61 @@ cleanup: virObjectUnref(cfg); } +static int +qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + bool cold_boot) +{ +char uuid[VIR_UUID_STRING_BUFLEN]; +virDomainEventPtr event = NULL; +int startupPolicy = disk-startupPolicy; + +virUUIDFormat(vm-def-uuid, uuid); + +switch ((enum virDomainStartupPolicy) startupPolicy) { +case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL: +break; + +case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: +virReportSystemError(errno, + _(cannot access file '%s'), + disk-src); +goto error; +break; + +case VIR_DOMAIN_STARTUP_POLICY_REQUISITE: +if (cold_boot) { +virReportSystemError(errno, + _(cannot access file '%s'), + disk-src); +goto error; +} +break; + +case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: +case VIR_DOMAIN_STARTUP_POLICY_LAST: +/* this should never happen */ +break; +} + +VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') + due to inaccessible source '%s', + disk-dst, vm-def-name, uuid, disk-src); + +event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, disk-info.alias, + VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); +if (event) +qemuDomainEventQueue(driver, event); + +VIR_FREE(disk-src); + +return 0; + +error: +return -1; +} + int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -2034,12 +2089,8 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, int ret = -1; size_t i; virDomainDiskDefPtr disk; -char uuid[VIR_UUID_STRING_BUFLEN]; -virDomainEventPtr event = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); -virUUIDFormat(vm-def-uuid, uuid); - for (i = 0; i vm-def-ndisks; i++) { disk = vm-def-disks[i]; @@ -2053,42 +2104,9 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, continue; } -switch ((enum virDomainStartupPolicy) disk-startupPolicy) { -case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL: -break; - -case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); -goto cleanup; -break; - -case VIR_DOMAIN_STARTUP_POLICY_REQUISITE: -if (cold_boot) { -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); -goto cleanup; -} -break; - -case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: -case VIR_DOMAIN_STARTUP_POLICY_LAST: -/* this should never happen */ -break; -} - -VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') - due to inaccessible source '%s', - disk-dst, vm-def-name, uuid, disk-src); - -event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, disk-info.alias, - VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); -if (event) -qemuDomainEventQueue(driver, event); - -VIR_FREE(disk-src); +if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + cold_boot) 0) +goto cleanup; } ret = 0; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] qemu: check presence of each disk and its backing file as well
For disk with startupPolicy support, such as cdrom and floppy when its chain is broken, the startup policy will apply, otherwise, report an error on chain issue. --- src/qemu/qemu_domain.c | 21 - src/qemu/qemu_process.c | 6 -- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index be77991..b607454 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2091,22 +2091,25 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virDomainDiskDefPtr disk; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +VIR_DEBUG(Checking for disk presence); for (i = 0; i vm-def-ndisks; i++) { disk = vm-def-disks[i]; -if (!disk-startupPolicy || !disk-src) +if (!disk-src) continue; -if (virFileAccessibleAs(disk-src, F_OK, -cfg-user, -cfg-group) = 0) { -/* disk accessible */ -continue; +if (qemuDomainDetermineDiskChain(driver, disk, false) = 0) +if (qemuDiskChainCheckBroken(disk) = 0) +continue; + +if (disk-startupPolicy) { +virResetLastError(); +if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + cold_boot) = 0) +continue; } -if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, - cold_boot) 0) -goto cleanup; +goto cleanup; } ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e8e459e..61a897c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3621,16 +3621,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuAssignDeviceAliases(vm-def, priv-qemuCaps) 0) goto cleanup; -VIR_DEBUG(Checking for CDROM and floppy presence); if (qemuDomainCheckDiskPresence(driver, vm, flags VIR_QEMU_PROCESS_START_COLD) 0) goto cleanup; -for (i = 0; i vm-def-ndisks; i++) { -if (qemuDomainDetermineDiskChain(driver, vm-def-disks[i], - false) 0) -goto cleanup; -} /* Get the advisory nodeset from numad if 'placement' of * either vcpu or numatune is 'auto'. -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] qemu: add helper functions for diskchain checking
*src/util/virstoragefile.c: Add a helper function to get the first name of missing backing files, if the name is NULL, it means the diskchain is not broken. *src/qemu/qemu_domain.c: qemuDiskChainCheckBroken(disk) to check if its chain is broken --- src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c| 22 ++ src/qemu/qemu_domain.h| 3 +++ src/util/virstoragefile.c | 47 +++ src/util/virstoragefile.h | 2 ++ 5 files changed, 75 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d9615ea..f7166d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1882,6 +1882,7 @@ virSocketAddrSetPort; # util/virstoragefile.h +virStorageFileChainGetBroken; virStorageFileChainLookup; virStorageFileFeatureTypeFromString; virStorageFileFeatureTypeToString; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 03a2aa6..be77991 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2187,6 +2187,28 @@ qemuDomainCleanupRun(virQEMUDriverPtr driver, } int +qemuDiskChainCheckBroken(virDomainDiskDefPtr disk) +{ +char *brokenFile = NULL; + +if (!disk-src || !disk-backingChain) +return 0; + +if (virStorageFileChainGetBroken(disk-backingChain, brokenFile) 0) +return -1; + +if (brokenFile) { +virReportError(VIR_ERR_INVALID_ARG, + _(Backing file '%s' of image '%s' is missing.), + brokenFile, disk-src); +VIR_FREE(brokenFile); +return -1; +} + +return 0; +} + +int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, bool force) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9a959d6..0a4a51e 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -347,6 +347,9 @@ bool qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv, int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virDomainObjPtr vm, bool start_with_state); + +int qemuDiskChainCheckBroken(virDomainDiskDefPtr disk); + int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, bool force); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cb6df91..e02e28a 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -572,6 +572,13 @@ virFindBackingFile(const char *start, bool start_is_dir, const char *path, goto cleanup; } +if (virFileAccessibleAs(combined, F_OK, getuid(), getgid()) 0) { +virReportSystemError(errno, + _(Cannot access backing file '%s'), + combined); +goto cleanup; +} + if (!(*canonical = canonicalize_file_name(combined))) { virReportSystemError(errno, _(Can't canonicalize path '%s'), path); @@ -1097,6 +1104,46 @@ virStorageFileGetMetadata(const char *path, int format, } /** + * virStorageFileChainCheckBroken + * + * Check whether CHAIN is broken, return the broken file name if yes, + * otherwise return NULL + */ +int +virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, + char **brokenFile) +{ +virStorageFileMetadataPtr tmp; +char *file = NULL; +int ret = -1; + +if (!chain) +return 0; + +*brokenFile = NULL; + +tmp = chain; +while (tmp) { +/* No backing store or backing store is not file */ + if (!tmp-backingStoreRaw) + break; + if (!tmp-backingStore) { + if (VIR_STRDUP(file, tmp-backingStoreRaw) 0) + goto error; + break; + } + tmp = tmp-backingMeta; +} + +*brokenFile = file; +ret = 0; + +error: +return ret; +} + + +/** * virStorageFileFreeMetadata: * * Free pointers in passed structure and structure itself. diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 4cb47e6..1f89839 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -90,6 +90,8 @@ virStorageFileMetadataPtr virStorageFileGetMetadata(const char *path, virStorageFileMetadataPtr virStorageFileGetMetadataFromFD(const char *path, int fd, int format); +int virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, + char **broken_file); const char *virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start, -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] qemu: report error if disk backing files doesn't exist
On 07/25/2013 10:17 PM, Martin Kletzander wrote: On 07/18/2013 01:32 PM, Guannan Ren wrote: s/doesn't/don't/ in $SUBJ Adding virFileAccessibleAs() to check if the backing file described in disk meta exist in real path. If not, report error. The uid and gid arguments don't take effect on F_OK mode for access, so use gid and gid of current process. This patch doesn't break anything new, but thanks to the getuid()/getgid(), it leaves the previous problem in the code. Even though F_OK doesn't need uid/gid to check whether the file exists, root may not have permissions for upper directories on NFS storage for example, so ... --- src/util/virstoragefile.c | 23 +++ tests/virstoragetest.c| 16 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cb6df91..b678eb8 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -572,6 +572,13 @@ virFindBackingFile(const char *start, bool start_is_dir, const char *path, goto cleanup; } +if (virFileAccessibleAs(combined, F_OK, getuid(), getgid()) 0) { +virReportSystemError(errno, + _(Backing file '%s' does not exist), + combined); +goto cleanup; +} + if (!(*canonical = canonicalize_file_name(combined))) { virReportSystemError(errno, _(Can't canonicalize path '%s'), path); ... this hunk does make the code report better errors, but in the future it should canonicalize the filename using root/qemu/domain users. ACK to this hunk, with the error reworded (e.g. Cannot access backing file %s) and, of course, commit message changed appropriately, but ... @@ -857,14 +864,10 @@ virStorageFileGetMetadataInternal(const char *path, !!directory, backing, meta-directory, meta-backingStore) 0) { -/* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ -meta-backingStoreIsFile = false; -backingFormat = VIR_STORAGE_FILE_NONE; -VIR_WARN(Backing file '%s' of image '%s' is missing., - meta-backingStoreRaw, path); - +VIR_FREE(backing); +VIR_ERROR(_(Backing file '%s' of image '%s' is missing.), + meta-backingStoreRaw, path); +goto cleanup; To fix a pre-existing error, we should (instead of this change) just add virResetLastError() here as the error is used somewhere else in the code and should be kept in virFindBackingFile(). Having it in the logs seems OK to me. It makes sense, but it seems that the tests/virstoragetest.c testcase is using the last error in checking warning flag, see: if (data-flags EXP_WARN) { if (!virGetLastError()) { fprintf(stderr, call should have warned\n); goto cleanup; } virResetLastError(); It is not serious issue without calling virResetLastError() here. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [v2 PATCH] caps: use -device for primary video when qemu =1.6
https://bugzilla.redhat.com/show_bug.cgi?id=981094 The commit 0ad9025ef introduce qemu flag QEMU_CAPS_DEVICE_VIDEO_PRIMARY for using -device VGA, -device cirrus-vga, -device vmware-svga and -device qxl-vga. In use, for -device qxl-vga, mouse doesn't display in guest window like the desciption in above bug. This patch try to use -device for primary video when qemu =1.6 which contains the bug fix patch --- src/qemu/qemu_capabilities.c | 6 +++--- tests/qemuhelptest.c | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5dc3c9e..08406b8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1189,8 +1189,6 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); } -if (version = 1002000) -virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); return 0; } @@ -2424,7 +2422,6 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT); virQEMUCapsSet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE); virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); -virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); } /* Capabilities that are architecture depending @@ -2597,6 +2594,9 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, if (qemuCaps-version = 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET); +if (qemuCaps-version = 1006000) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) 0) goto cleanup; if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) 0) diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index d2fd794..d6cc04b 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -929,7 +929,6 @@ mymain(void) QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, -QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_USB_SERIAL, QEMU_CAPS_DEVICE_USB_NET, QEMU_CAPS_DTB, @@ -1041,7 +1040,6 @@ mymain(void) QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, -QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_USB_SERIAL, QEMU_CAPS_DEVICE_USB_NET, QEMU_CAPS_DTB, -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Don't overwrite errors in qemuTranslateDiskSourcePool
On 07/24/2013 04:43 PM, Ján Tomko wrote: Both virStoragePoolFree and virStorageVolFree reset the last error, which might lead to the cryptic message: An error occurred, but the cause is unknown When the volume wasn't found, virStorageVolFree was called with NULL, leading to an error: invalid storage volume pointer in virStorageVolFree This patch changes it to: Storage volume not found: no storage vol with matching name 'tomato' --- src/qemu/qemu_conf.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3e7b78a..80b8156 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1248,6 +1248,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, char *poolxml = NULL; virStorageVolInfo info; int ret = -1; +virErrorPtr savedError; if (def-type != VIR_DOMAIN_DISK_TYPE_VOLUME) return 0; @@ -1324,8 +1325,14 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, def-srcpool-voltype = info.type; ret = 0; cleanup: -virStoragePoolFree(pool); -virStorageVolFree(vol); +savedError = virSaveLastError(); It would be better to save error only when ret 0. In other place, we use format like this: if (ret 0 !savedError) savedError = virSaveLastError(); virStoragePoolFree(pool); virStorageVolFree(vol); if (savedError) { virSetError(orig_err); virFreeError(orig_err); } Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] caps: use -device for primary video when qemu =1.6
https://bugzilla.redhat.com/show_bug.cgi?id=981094 The commit 0ad9025ef introduce qemu flag QEMU_CAPS_DEVICE_VIDEO_PRIMARY for using -device VGA, -device cirrus-vga, -device vmware-svga and -device qxl-vga. In use, for -device qxl-vga, mouse doesn't display in guest window like the desciption in above bug. This patch try to use -device for primary video when qemu =1.6 which is safe. --- src/qemu/qemu_capabilities.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5dc3c9e..08406b8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1189,8 +1189,6 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); } -if (version = 1002000) -virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); return 0; } @@ -2424,7 +2422,6 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT); virQEMUCapsSet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE); virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); -virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); } /* Capabilities that are architecture depending @@ -2597,6 +2594,9 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, if (qemuCaps-version = 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET); +if (qemuCaps-version = 1006000) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) 0) goto cleanup; if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) 0) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix change-media bug on disk block type
On 07/23/2013 12:20 AM, Eric Blake wrote: On 07/22/2013 01:40 AM, Guannan Ren wrote: Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=923053 When cdrom is block type, the virsh change-media failed to insert source info because virsh uses source block='/dev/sdb'/ while the correct name of the attribute for block disks is dev. --- tools/virsh-domain.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) ACK. diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 606bcdf..8cafce4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9866,8 +9866,10 @@ vshPrepareDiskXML(xmlNodePtr disk_node, if (source) { new_node = xmlNewNode(NULL, BAD_CAST source); -xmlNewProp(new_node, (const xmlChar *)disk_type, - (const xmlChar *)source); +if (STREQ(disk_type, block)) +xmlNewProp(new_node, BAD_CAST dev, BAD_CAST source); +else +xmlNewProp(new_node, BAD_CAST disk_type, BAD_CAST source); xmlAddChild(disk_node, new_node); } else if (type == VSH_PREPARE_DISK_XML_INSERT) { vshError(NULL, _(No source is specified for inserting media)); Thanks and pushed Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: fix change-media bug on disk block type
Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=923053 When cdrom is block type, the virsh change-media failed to insert source info because virsh uses source block='/dev/sdb'/ while the correct name of the attribute for block disks is dev. --- tools/virsh-domain.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 606bcdf..8cafce4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9866,8 +9866,10 @@ vshPrepareDiskXML(xmlNodePtr disk_node, if (source) { new_node = xmlNewNode(NULL, BAD_CAST source); -xmlNewProp(new_node, (const xmlChar *)disk_type, - (const xmlChar *)source); +if (STREQ(disk_type, block)) +xmlNewProp(new_node, BAD_CAST dev, BAD_CAST source); +else +xmlNewProp(new_node, BAD_CAST disk_type, BAD_CAST source); xmlAddChild(disk_node, new_node); } else if (type == VSH_PREPARE_DISK_XML_INSERT) { vshError(NULL, _(No source is specified for inserting media)); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH V3] Modify repos/network/network_list.py and add network_list case to conf
On 07/22/2013 03:53 PM, hongming zhang wrote: Modify the old network_list.py. The new network_list.py covers all flags of listAllNetworks and the following api. and add network_list to basic_network.conf. virNetwork: name() isActive() isPersistent() virConnect: listAllNetworks() V1- V2 1.Change the flag in conf from digit to string 2.Remove the checking method using the result of virsh with flags 3.Add checking method via checking network autostart dir 4.Modify the basic_network.conf V2 - V3 1.Remove check_bridge_ip method 2.Remove network.bridgeName() call 3.Fix a typo in log --- cases/basic_network.conf | 35 +- repos/network/network_list.py | 260 - 2 files changed, 130 insertions(+), 165 deletions(-) diff --git a/cases/basic_network.conf b/cases/basic_network.conf index 991ad99..91d7f21 100644 --- a/cases/basic_network.conf +++ b/cases/basic_network.conf @@ -14,10 +14,18 @@ network:define netmode nat +network:network_list +flags + inactive + network:start networkname $defaultnetname +network:network_list +flags + active + network:autostart networkname $defaultnetname @@ -28,6 +36,10 @@ network:destroy networkname $defaultnetname +network:network_list +flags + default + network:undefine networkname $defaultnetname @@ -48,6 +60,10 @@ network:create netmode nat +network:network_list +flags +transient + network:destroy networkname $defaultnetname @@ -68,6 +84,10 @@ network:define netmode route +network:network_list +flags + persistent + network:start networkname $defaultnetname @@ -106,7 +126,6 @@ network:destroy networkname $defaultnetname - network:define networkname $defaultnetname @@ -127,12 +146,20 @@ network:start networkname $defaultnetname +network:network_list +flags + noautostart + network:autostart networkname $defaultnetname autostart enable +network:network_list +flags + autostart + network:destroy networkname $defaultnetname @@ -141,7 +168,6 @@ network:undefine networkname $defaultnetname - network:create networkname $defaultnetname @@ -162,8 +188,3 @@ network:destroy networkname $defaultnetname - - - - - diff --git a/repos/network/network_list.py b/repos/network/network_list.py index 7c34f69..b94b505 100644 --- a/repos/network/network_list.py +++ b/repos/network/network_list.py @@ -1,184 +1,128 @@ #!/usr/bin/env python # To test virsh net-list command -import os -import sys -import re -import commands - import libvirt from libvirt import libvirtError from src import sharedmod from utils import utils -required_params = ('netlistopt',) +required_params = ('flags',) optional_params = {} -VIRSH_QUIET_NETLIST = virsh --quiet net-list %s|awk '{print $1}' -VIRSH_NETLIST = virsh net-list %s -GET_BRIDGE_IP = /sbin/ifconfig %s | grep 'inet addr:' | cut -d: -f2 | awk '{print $1}' -CONFIG_DIR = /etc/libvirt/qemu/networks/ +LS_NETWORK_DIR = ls /etc/libvirt/qemu/networks/ +LS_AUTOSTART_NET = ls /etc/libvirt/qemu/networks/autostart/ -def get_option_list(params): -return options we need to test +def check_persistent_netxml(networkname): -logger = params['logger'] -option_list=[] - -value = params['netlistopt'] - -if value == 'all': -option_list = [' ', '--all', '--inactive'] -elif value == '--all' or value == '--inactive': -option_list.append(value) -else: -logger.error(value %s is not supported % value) -return 1, option_list - -return 0, option_list - -def get_output(logger, command, flag): -execute shell command +Check if the network is persistent via checking network xml dir +if the network is persistent, return True, or return False -status, ret = commands.getstatusoutput(command) -if not flag and status: -logger.error(executing + \ + command + \ + failed) -logger.error(ret) -return status, ret - -def check_all_option(conn, logger): -check the output of virsh net-list with --all option - -all_network = [] -entries = os.listdir(CONFIG_DIR) -logger.debug(%s in %s % (entries, CONFIG_DIR)) -status, network_names = get_output(logger, VIRSH_QUIET_NETLIST % '--all', 0) -if not status: -all_network = network_names.split('\n') -logger.info(all network is %s % all_network) -else: -return 1 - -if all_network == ['']: -return 0 -for entry in entries: -if not entry.endswith('.xml'): -continue +(status, output) = utils.exec_cmd(LS_NETWORK_DIR, shell=True) +network_list_dir = [] +if
Re: [libvirt] [test-API][PATCH V2] Modify repos/network/network_list.py and add network_list case to conf
On 07/17/2013 04:46 PM, hongming zhang wrote: Modify the old network_list.py. The new network_list.py covers all flags of listAllNetworks and the following api. and add network_list to basic_network.conf. virNetwork: name() bridgeName() isActive() isPersistent() virConnect: listAllNetworks() Changes from V2 to V1 as follows 1.Change the flag in conf from digit to string 2.Remove the checking method using the result of virsh with flags 3.Add checking method via checking network autostart dir 4.Modify the basic_network.conf --- cases/basic_network.conf | 35 - repos/network/network_list.py | 277 ++--- 2 files changed, 148 insertions(+), 164 deletions(-) diff --git a/cases/basic_network.conf b/cases/basic_network.conf index 991ad99..91d7f21 100644 --- a/cases/basic_network.conf +++ b/cases/basic_network.conf @@ -14,10 +14,18 @@ network:define netmode nat +network:network_list +flags + inactive + network:start networkname $defaultnetname +network:network_list +flags + active + network:autostart networkname $defaultnetname @@ -28,6 +36,10 @@ network:destroy networkname $defaultnetname +network:network_list +flags + default + network:undefine networkname $defaultnetname @@ -48,6 +60,10 @@ network:create netmode nat +network:network_list +flags +transient + network:destroy networkname $defaultnetname @@ -68,6 +84,10 @@ network:define netmode route +network:network_list +flags + persistent + network:start networkname $defaultnetname @@ -106,7 +126,6 @@ network:destroy networkname $defaultnetname - network:define networkname $defaultnetname @@ -127,12 +146,20 @@ network:start networkname $defaultnetname +network:network_list +flags + noautostart + network:autostart networkname $defaultnetname autostart enable +network:network_list +flags + autostart + network:destroy networkname $defaultnetname @@ -141,7 +168,6 @@ network:undefine networkname $defaultnetname - network:create networkname $defaultnetname @@ -162,8 +188,3 @@ network:destroy networkname $defaultnetname - - - - - diff --git a/repos/network/network_list.py b/repos/network/network_list.py index 7c34f69..175d6fa 100644 --- a/repos/network/network_list.py +++ b/repos/network/network_list.py @@ -1,184 +1,147 @@ #!/usr/bin/env python # To test virsh net-list command -import os -import sys -import re -import commands - import libvirt from libvirt import libvirtError from src import sharedmod from utils import utils -required_params = ('netlistopt',) +required_params = ('flags',) optional_params = {} -VIRSH_QUIET_NETLIST = virsh --quiet net-list %s|awk '{print $1}' -VIRSH_NETLIST = virsh net-list %s -GET_BRIDGE_IP = /sbin/ifconfig %s | grep 'inet addr:' | cut -d: -f2 | awk '{print $1}' -CONFIG_DIR = /etc/libvirt/qemu/networks/ - -def get_option_list(params): -return options we need to test - -logger = params['logger'] -option_list=[] - -value = params['netlistopt'] - -if value == 'all': -option_list = [' ', '--all', '--inactive'] -elif value == '--all' or value == '--inactive': -option_list.append(value) +VIRSH_NETWORK_LIST = virsh net-list %s|sed -n '3,$'p|awk '{print $1}' +GET_BRIDGE_IP = /sbin/ifconfig %s | grep 'inet addr:' | cut -d: -f2 | awk \ +'{print $1}' +LS_NETWORK_DIR = ls /etc/libvirt/qemu/networks/ +LS_AUTOSTART_NET = ls /etc/libvirt/qemu/networks/autostart/ + +def check_bridge_ip(bridgename): + Check if the bridge has ip + +(status, output) = utils.exec_cmd(GET_BRIDGE_IP % bridgename,\ + shell=True) +if not status and utils.do_ping(output[0], 50): +logger.info(Bridge %s is active % bridgename) +logger.info(%s has ip: %s % (bridgename, output[0])) GET_BRIDGE_IP is not a common solution to extract ip In fedora, it is inet xxx.xxx.xxx.xxx which is not the same as inet addr: xxx.xxx.xxx.xxx on RHEL +return True else: -logger.error(value %s is not supported % value) -return 1, option_list - -return 0, option_list +logger.error(Bridge %s has no ip or fails to ping % bridgename) +return False -def get_output(logger, command, flag): -execute shell command +def check_persistent_netxml(networkname): -status, ret = commands.getstatusoutput(command) -if not flag and status: -logger.error(executing + \ + command + \ + failed) -logger.error(ret) -return status, ret - -def check_all_option(conn, logger): -check the output of virsh net-list
[libvirt] [PATCH 1/4] qemu: refactor qemuDomainCheckDiskPresence for only disk presence check
Refactor this function to make it focus on disk presence checking, including diskchain checking, and not only for CDROM and Floppy. This change is good for the following patches. --- src/qemu/qemu_domain.c | 99 ++ 1 file changed, 59 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 06efe14..1fa1a5f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1989,6 +1989,61 @@ cleanup: virObjectUnref(cfg); } +static int +qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + int startupPolicy, + bool cold_boot) +{ +char uuid[VIR_UUID_STRING_BUFLEN]; +virDomainEventPtr event = NULL; + +virUUIDFormat(vm-def-uuid, uuid); + +switch ((enum virDomainStartupPolicy) startupPolicy) { +case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL: +break; + +case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: +virReportSystemError(errno, + _(cannot access file '%s'), + disk-src); +goto error; +break; + +case VIR_DOMAIN_STARTUP_POLICY_REQUISITE: +if (cold_boot) { +virReportSystemError(errno, + _(cannot access file '%s'), + disk-src); +goto error; +} +break; + +case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: +case VIR_DOMAIN_STARTUP_POLICY_LAST: +/* this should never happen */ +break; +} + +VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') + due to inaccessible source '%s', + disk-dst, vm-def-name, uuid, disk-src); + +event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, disk-info.alias, + VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); +if (event) +qemuDomainEventQueue(driver, event); + +VIR_FREE(disk-src); + +return 0; + +error: +return -1; +} + int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -1997,12 +2052,8 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, int ret = -1; size_t i; virDomainDiskDefPtr disk; -char uuid[VIR_UUID_STRING_BUFLEN]; -virDomainEventPtr event = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); -virUUIDFormat(vm-def-uuid, uuid); - for (i = 0; i vm-def-ndisks; i++) { disk = vm-def-disks[i]; @@ -2016,42 +2067,10 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, continue; } -switch ((enum virDomainStartupPolicy) disk-startupPolicy) { -case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL: -break; - -case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); -goto cleanup; -break; - -case VIR_DOMAIN_STARTUP_POLICY_REQUISITE: -if (cold_boot) { -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); -goto cleanup; -} -break; - -case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: -case VIR_DOMAIN_STARTUP_POLICY_LAST: -/* this should never happen */ -break; -} - -VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') - due to inaccessible source '%s', - disk-dst, vm-def-name, uuid, disk-src); - -event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, disk-info.alias, - VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); -if (event) -qemuDomainEventQueue(driver, event); - -VIR_FREE(disk-src); +if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + disk-startupPolicy, + cold_boot) 0) +goto cleanup; } ret = 0; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] qemu: check presence of each disk in chain
For disk with startupPolicy support, such as cdrom and floppy when its chain is broken, the startup policy will apply, otherwise, report error on chain issue. Force to collect diskchain metadata when qemu process start everytime. --- src/qemu/qemu_domain.c | 19 ++- src/qemu/qemu_process.c | 7 --- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1fa1a5f..c4adbaa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2057,20 +2057,21 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, for (i = 0; i vm-def-ndisks; i++) { disk = vm-def-disks[i]; -if (!disk-startupPolicy || !disk-src) +if (!disk-src) continue; -if (virFileAccessibleAs(disk-src, F_OK, -cfg-user, -cfg-group) = 0) { -/* disk accessible */ +if (qemuDomainDetermineDiskChain(driver, + disk, true) = 0) continue; + +if (disk-startupPolicy) { +if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + disk-startupPolicy, + cold_boot) = 0) +continue; } -if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk, - disk-startupPolicy, - cold_boot) 0) -goto cleanup; +goto cleanup; } ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3d5e8f6..f495267 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3562,17 +3562,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuAssignDeviceAliases(vm-def, priv-qemuCaps) 0) goto cleanup; -VIR_DEBUG(Checking for CDROM and floppy presence); if (qemuDomainCheckDiskPresence(driver, vm, flags VIR_QEMU_PROCESS_START_COLD) 0) goto cleanup; -for (i = 0; i vm-def-ndisks; i++) { -if (qemuDomainDetermineDiskChain(driver, vm-def-disks[i], - false) 0) -goto cleanup; -} - /* Get the advisory nodeset from numad if 'placement' of * either vcpu or numatune is 'auto'. */ -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] fix two issues about diskchain
1, Report an backing missing error on guest boot if its disks have broken backing file chain 2, List volume even if its diskchain is broken Guannan Ren(4) [PATCH 1/4] qemu: refactor qemuDomainCheckDiskPresence for only disk [PATCH 2/4] qemu: report error if disk backing files doesn't exist [PATCH 3/4] qemu: check presence of each disk in chain [PATCH 4/4] storage: list volumes even if its diskchain is broken src/qemu/qemu_domain.c | 108 src/qemu/qemu_process.c | 7 --- src/storage/storage_backend_fs.c | 4 +++- src/util/virstoragefile.c| 23 +++ tests/virstoragetest.c | 16 5 files changed, 90 insertions(+), 68 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] qemu: report error if disk backing files doesn't exist
Adding virFileAccessibleAs() to check if the backing file described in disk meta exist in real path. If not, report error. The uid and gid arguments don't take effect on F_OK mode for access, so use gid and gid of current process. --- src/util/virstoragefile.c | 23 +++ tests/virstoragetest.c| 16 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cb6df91..b678eb8 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -572,6 +572,13 @@ virFindBackingFile(const char *start, bool start_is_dir, const char *path, goto cleanup; } +if (virFileAccessibleAs(combined, F_OK, getuid(), getgid()) 0) { +virReportSystemError(errno, + _(Backing file '%s' does not exist), + combined); +goto cleanup; +} + if (!(*canonical = canonicalize_file_name(combined))) { virReportSystemError(errno, _(Can't canonicalize path '%s'), path); @@ -857,14 +864,10 @@ virStorageFileGetMetadataInternal(const char *path, !!directory, backing, meta-directory, meta-backingStore) 0) { -/* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ -meta-backingStoreIsFile = false; -backingFormat = VIR_STORAGE_FILE_NONE; -VIR_WARN(Backing file '%s' of image '%s' is missing., - meta-backingStoreRaw, path); - +VIR_FREE(backing); +VIR_ERROR(_(Backing file '%s' of image '%s' is missing.), + meta-backingStoreRaw, path); +goto cleanup; } } VIR_FREE(backing); @@ -1047,6 +1050,10 @@ virStorageFileGetMetadataRecurse(const char *path, const char *directory, uid, gid, allow_probe, cycle); +if (!ret-backingMeta) { +virStorageFileFreeMetadata(ret); +ret = NULL; +} } return ret; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index a3c59ef..030f81b 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -480,10 +480,10 @@ mymain(void) /* Qcow2 file with missing backing file but specified type */ const testFileData chain9[] = { qcow2_bogus }; TEST_CHAIN(9, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2, - chain9, EXP_WARN, - chain9, ALLOW_PROBE | EXP_WARN, - chain9, EXP_WARN, - chain9, ALLOW_PROBE | EXP_WARN); + chain9, EXP_FAIL, + chain9, ALLOW_PROBE | EXP_FAIL, + chain9, EXP_FAIL, + chain9, ALLOW_PROBE | EXP_FAIL); /* Rewrite qcow2 to a missing backing file, without backing type */ virCommandFree(cmd); @@ -495,10 +495,10 @@ mymain(void) /* Qcow2 file with missing backing file and no specified type */ const testFileData chain10[] = { qcow2_bogus }; TEST_CHAIN(10, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2, - chain10, EXP_WARN, - chain10, ALLOW_PROBE | EXP_WARN, - chain10, EXP_WARN, - chain10, ALLOW_PROBE | EXP_WARN); + chain10, EXP_FAIL, + chain10, ALLOW_PROBE | EXP_FAIL, + chain10, EXP_FAIL, + chain10, ALLOW_PROBE | EXP_FAIL); /* Rewrite qcow2 to use an nbd: protocol as backend */ virCommandFree(cmd); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] storage: list volumes even if its diskchain is broken
--- src/storage/storage_backend_fs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d305b06..c6c019d 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -94,7 +94,9 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, if (!(meta = virStorageFileGetMetadataFromFD(target-path, fd, target-format))) { -ret = -1; +/* list volume even if its diskchain is broken */ +virResetLastError(); +ret = -3; goto error; } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [v3 PATCH] storage: skip async-deleted volume while fs storage is being refreshed
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=977706 v1, v2 by Ján Tomko https://www.redhat.com/archives/libvir-list/2013-July/msg00639.html To get volume file fd in the fist place. The purpose of doing so is to ensure the async deletion of volume doesn't make impact on the following operation for the volume till we recheck the existence of volume file later. If we don't get its fd firstly, it is diffcult to differentiate the non-existent file error caused by system or being deleted asynchronous. --- src/storage/storage_backend.c| 49 +--- src/storage/storage_backend_fs.c | 6 + 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 8d5880e..aa1635a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1093,29 +1093,13 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) int fd, mode = 0; struct stat sb; char *base = last_component(path); - -if (lstat(path, sb) 0) { -virReportSystemError(errno, - _(cannot stat file '%s'), - path); -return -1; -} - -if (S_ISFIFO(sb.st_mode)) { -VIR_WARN(ignoring FIFO '%s', path); -return -2; -} else if (S_ISSOCK(sb.st_mode)) { -VIR_WARN(ignoring socket '%s', path); -return -2; -} +int ret = -1; if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) 0) { -if ((errno == ENOENT || errno == ELOOP) -S_ISLNK(sb.st_mode)) { -VIR_WARN(ignoring dangling symlink '%s', path); +if (errno == ENOENT) { +VIR_WARN(volume '%s' does not exist, path); return -2; } - virReportSystemError(errno, _(cannot open volume '%s'), path); @@ -1126,8 +1110,7 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) virReportSystemError(errno, _(cannot stat file '%s'), path); -VIR_FORCE_CLOSE(fd); -return -1; +goto error; } if (S_ISREG(sb.st_mode)) @@ -1141,26 +1124,36 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) if (STREQ(base, .) || STREQ(base, ..)) { -VIR_FORCE_CLOSE(fd); VIR_INFO(Skipping special dir '%s', base); -return -2; +goto skipfile; } +} else if (S_ISFIFO(sb.st_mode)) { +VIR_WARN(ignoring FIFO '%s', path); +goto skipfile; +} else if (S_ISSOCK(sb.st_mode)) { +VIR_WARN(ignoring socket '%s', path); +goto skipfile; } if (!(mode flags)) { -VIR_FORCE_CLOSE(fd); -VIR_INFO(Skipping volume '%s', path); - if (mode VIR_STORAGE_VOL_OPEN_ERROR) { virReportError(VIR_ERR_INTERNAL_ERROR, _(unexpected storage mode for '%s'), path); -return -1; +goto error; } -return -2; +VIR_INFO(Skipping volume '%s', path); +goto skipfile; } return fd; + +skipfile: +ret = -2; + +error: +VIR_FORCE_CLOSE(fd); +return ret; } int virStorageBackendVolOpen(const char *path) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d305b06..d0276fc 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -889,6 +889,12 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, } } +/* Recheck the existence of volume again */ +if (access(vol-target.path, F_OK) 0) { +virStorageVolDefFree(vol); +vol = NULL; +continue; +} if (VIR_REALLOC_N(pool-volumes.objs, pool-volumes.count+1) 0) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cgroup: Free line even if no characters were read
On 07/16/2013 08:14 PM, Ján Tomko wrote: Even if getline doesn't read any characters it allocates a buffer. ==404== 120 bytes in 1 blocks are definitely lost in loss record 1,344 of 1,671 ==404==at 0x4C2C71B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==404==by 0x906F862: getdelim (iogetdelim.c:68) ==404==by 0x52A48FB: virCgroupPartitionNeedsEscaping (vircgroup.c:1136) ==404==by 0x52A0FB4: virCgroupPartitionEscape (vircgroup.c:1171) ==404==by 0x52A0EA4: virCgroupNewDomainPartition (vircgroup.c:1450) Introduced by f366273. --- Can STRPREFIX(path, line) be possibly true if tmp is NULL? path[NULL - line] would be accessed in that case. src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 5a98393..2419d80 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1136,38 +1136,38 @@ static int virCgroupPartitionNeedsEscaping(const char *path) while (getline(line, len, fp) 0) { if (STRPREFIX(line, #subsys_name)) { VIR_FREE(line); continue; } char *tmp = strchr(line, ' '); if (tmp) *tmp = '\0'; len = tmp - line; if (STRPREFIX(path, line) path[len] == '.') { ret = 1; -VIR_FREE(line); goto cleanup; } VIR_FREE(line); } if (ferror(fp)) { ret = -EIO; goto cleanup; } cleanup: +VIR_FREE(line); VIR_FORCE_FCLOSE(fp); return ret; } static int virCgroupPartitionEscape(char **path) { size_t len = strlen(*path) + 1; int rc; char escape = '_'; if ((rc = virCgroupPartitionNeedsEscaping(*path)) = 0) return rc; ACK, I can reproduce the memory leak. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/4] storage: report error rather than warning if backing files doesn't exist
On 07/16/2013 04:40 PM, Daniel P. Berrange wrote: On Tue, Jul 16, 2013 at 08:14:54AM +0200, Martin Kletzander wrote: On 07/15/2013 05:31 PM, Ján Tomko wrote: On 07/02/2013 11:35 AM, Guannan Ren wrote: If one of backing files for disk source doesn't exist, the guest will not be able to find and use the disk even though the disk still exists in guest xml definition. So reporting an error make more sense. Adding virFileAccessibleAs() to check if the backing file described in disk meta exist in real path. If not, report error. the uid and gid arguments don't have so much meannings for F_OK, so give 0 for them. --- src/util/virstoragefile.c | 23 +++ tests/virstoragetest.c| 16 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 27aa4fe..cb61e5b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -870,14 +877,10 @@ virStorageFileGetMetadataInternal(const char *path, !!directory, backing, meta-directory, meta-backingStore) 0) { -/* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ -meta-backingStoreIsFile = false; -backingFormat = VIR_STORAGE_FILE_NONE; -VIR_WARN(Backing file '%s' of image '%s' is missing., - meta-backingStoreRaw, path); - +VIR_ERROR(_(Backing file '%s' of image '%s' is missing.), + meta-backingStoreRaw, path); +VIR_FREE(backing); +goto cleanup; } } VIR_FREE(backing); This change means you won't be able to start the pool if one of the files is missing a backing file. I've forwarded a patch [1] from/for [2] that ignores missing files on pool start and there is a bug [3] requesting that we ignore other files as well. I feel like this is going in the other direction. Wouldn't it be enough to check for them on domain start-up and leave the pool running even if one of those volumes doesn't have an existing backing file? How about making it configurable for the pool? There are definitely some users who want the pool to reflect actual info after pool-refresh. I don't think this needs to be configurable. The pool should show *every* single file, regardless of whether the file has a broken backing file. We shouldn't be trying to second guess what the user wants to do with a image with broken backing file. Just expose as much info as we have and let them deal with the problem. Daniel, How about for guest cold bootup in this situation where one of its disks has a broken file we should throw an error out or give a warn in the log as what it is doing currently. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH] Modify repos/network/network_list.py and add network_list case to conf
On 07/16/2013 05:38 PM, hongming zhang wrote: Modify the old network_list.py. The new network_list.py covers all flags of listAllNetworks and the following api. and add network_list to basic_network.conf. virNetwork: name() bridgeName() isActive() isPersistent() virConnect: listAllNetworks() --- cases/basic_network.conf | 37 +- repos/network/network_list.py | 279 ++--- 2 files changed, 151 insertions(+), 165 deletions(-) diff --git a/cases/basic_network.conf b/cases/basic_network.conf index 991ad99..805cfd0 100644 --- a/cases/basic_network.conf +++ b/cases/basic_network.conf @@ -14,6 +14,16 @@ network:define netmode nat +#VIR_CONNECT_LIST_NETWORKS_INACTIVE = 1 +#VIR_CONNECT_LIST_NETWORKS_ACTIVE = 2 +#VIR_CONNECT_LIST_NETWORKS_PERSISTENT = 4 +#VIR_CONNECT_LIST_NETWORKS_TRANSIENT = 8 +#VIR_CONNECT_LIST_NETWORKS_AUTOSTART = 16 +#VIR_CONNECT_LIST_NETWORKS_NO_AUTOSTART = 32 About the list all networks testcase, we can divide the about six flags into three groups 1 inactive/active 2 persistent/transistent 3 autostart/non-autostart we just test the result with these three flags Currently, the case compares the results of 'virsh network-list' to the results of calling python APIs it is not so necessary because both virsh and python bindings calls the same libvirt shared library. we can test the virsh tool by using shell scripts in other tests place. After getting the result from python API, checking its validation in /etc/libvirt/qemu/networks/ make the testcase looks good. +network:network_list +flags + 1 + using string here is better than digit. The conversion between digit to string is a little complicated. The simpler, the better. So finally it comes to four tesecases. 1 list all network(default flags == 0) 2 list inactive/active networks 3,list persistent/transistent networks 4,list autostart/non-autostart networks. Or, go further, we can pick up one of networks to change its state, then see the changes in the output of this list. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/4] storage: report error rather than warning if backing files doesn't exist
On 07/15/2013 11:31 PM, Ján Tomko wrote: On 07/02/2013 11:35 AM, Guannan Ren wrote: If one of backing files for disk source doesn't exist, the guest will not be able to find and use the disk even though the disk still exists in guest xml definition. So reporting an error make more sense. Adding virFileAccessibleAs() to check if the backing file described in disk meta exist in real path. If not, report error. the uid and gid arguments don't have so much meannings for F_OK, so give 0 for them. --- src/util/virstoragefile.c | 23 +++ tests/virstoragetest.c| 16 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 27aa4fe..cb61e5b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -870,14 +877,10 @@ virStorageFileGetMetadataInternal(const char *path, !!directory, backing, meta-directory, meta-backingStore) 0) { -/* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ -meta-backingStoreIsFile = false; -backingFormat = VIR_STORAGE_FILE_NONE; -VIR_WARN(Backing file '%s' of image '%s' is missing., - meta-backingStoreRaw, path); - +VIR_ERROR(_(Backing file '%s' of image '%s' is missing.), + meta-backingStoreRaw, path); +VIR_FREE(backing); +goto cleanup; } } VIR_FREE(backing); This change means you won't be able to start the pool if one of the files is missing a backing file. I've forwarded a patch [1] from/for [2] that ignores missing files on pool start and there is a bug [3] requesting that we ignore other files as well. I feel like this is going in the other direction. Wouldn't it be enough to check for them on domain start-up and leave the pool running even if one of those volumes doesn't have an existing backing file? Jan [1] https://www.redhat.com/archives/libvir-list/2013-July/msg00635.html [2] https://bugzilla.redhat.com/show_bug.cgi?id=977706 [3] https://bugzilla.redhat.com/show_bug.cgi?id=710866 As what you pointed out is close to my issue, I chose to propose a patch on bug 977706 first https://www.redhat.com/archives/libvir-list/2013-July/msg01026.html. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] python: return dictionay without value in case of no blockjob
On 07/13/2013 01:36 AM, Michal Privoznik wrote: On 17.05.2013 08:30, Guannan Ren wrote: s/dictionay/dictionary/ in $SUBJ Currently, when there is no blockjob, dom.blockJobInfo('vda') still reports error because it didn't distinguish return value 0 from -1. s/didn't/doesn't/ libvirt.libvirtError: virDomainGetBlockJobInfo() failed virDomainGetBlockJobInfo() API return value: -1 in case of failure, 0 when nothing found, 1 found. And use PyDict_SetItemString instead of PyDict_SetItem when key is string type. PyDict_SetItemString increments key/value reference s/string type/of string type/ count, so calling Py_DECREF() for value. For key, we don't need to s/calling/call/ do this, because PyDict_SetItemString will handle it internally. --- python/libvirt-override.c | 54 ++- 1 file changed, 39 insertions(+), 15 deletions(-) ACK with the commit message fixed. Michal Thanks for this review. I pushed with these fixed Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/4] Add startupPolicy attribute support for hard disks
On 07/02/2013 05:35 PM, Guannan Ren wrote: v3: https://www.redhat.com/archives/libvir-list/2013-June/thread.html v3 to v4: Rebase v2 to v3: Not only check disk source, startupPolicy should work if any backing file is missing. The commit 039a3283 break the limition of contiguous device boot orders, so I remove my previous patch about it. v1 to v2: Added relax schema for disk of block and dir type Removed original patch 3/5. The set of patches is trying to add 'startupPolicy' attribute support to the source element of hard disks. Policy levels are using the mandatory, requisite, optional levels as originally documented. For the 'optional' policy, there is a little difference from CDROM and Floppy which only drop its source path, for disks, if missing, the checking function will drop their definitions, because qemu doesn't allow missing source path for hard disk. Are we going to support this feature. I think this is a good feature for two reasons: 1, currently, we only check the disk presence for floppy and CDROM device which is not enough. For disks with backing files, if one of its backing file is missing, libvirt doesn't check anything, the qemu will throw an unclear error message as follows: A guest with a disk image /var/lib/libvirt/images/snapshot/snap2.qcow # qemu-img info --backing-chain /var/lib/libvirt/images/snapshot/snap2.qcow image: /var/lib/libvirt/images/snapshot/snap2.qcow file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 19M cluster_size: 65536 backing file: /var/lib/libvirt/images/snapshot/snap1.qcow backing file format: qcow2 image: /var/lib/libvirt/images/snapshot/snap1.qcow file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 196K cluster_size: 65536 backing file: /var/lib/libvirt/images/fedora18.img backing file format: raw if we change snap1.qcow to *snap1.qcow.back* , then bootup the guest The qemu will throw an error: qemu-system-x86_64: -drive file=/var/lib/libvirt/images/snapshot/snap2.qcow,if=none,id=drive-virtio-disk0,format=qcow2: could not open disk image /var/lib/libvirt/images/snapshot/snap2.qcow: No such file or directory Actually, snap2.qcow is there, one of its backing file snap1.qcow is missing. So we need to check the presence of all files in disk chains. With this patch, libvirt will throw an error: error: Backing file '/var/lib/libvirt/images/snapshot/snap1.qcow' does not exist: No such file or directory 2, Adding 'startupPolicy' attribute for guest disk is useful for guests which use the disk from storage-centric production environment. Guest uses the the block disk assigned by Fibre channel or FCoE or ISCSI storage. The storage administrator can adjust the disk assignment between guests, if guest disk with startupPolicy value of optional, guest still can boot up after its assigned disks are missing. So I think it is good to add this flexibility to disk configuration for guests. Any thoughts? Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/4] conf: add startupPolicy attribute for harddisk
On 07/15/2013 09:31 PM, Martin Kletzander wrote: On 07/02/2013 11:35 AM, Guannan Ren wrote: Add startupPolicy attribute policy for harddisk with type file, block and dir. The network type disk is still not supported. --- docs/schemas/domaincommon.rng | 6 ++ src/conf/domain_conf.c| 20 +--- 2 files changed, 19 insertions(+), 7 deletions(-) [...] diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 011de71..1040b40 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c [...] @@ -5410,11 +5410,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } -if (def-device != VIR_DOMAIN_DISK_DEVICE_CDROM -def-device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { +if (def-type == VIR_DOMAIN_DISK_TYPE_NETWORK) { virReportError(VIR_ERR_INVALID_ARG, Pre-existing, but shouldn't this be VIR_ERR_XML_ERROR ? Otherwise the patch looks ok, but is missing documentation. And in this case it is important to have it there, I think. The possibility of specifying startupPolicy should be also documented and it should be mentioned there that the whole disk will be dropped in case this attribute is specified and the disk (or any other in it's backing chain) is not found. I'm trying to stress this out because for cdrom and floppy disks we can safely drop the source only, it can be plugged in even with old QEMU, OSes are used to that. But with disks it changes what is seen from the guest. I believe that's also why startupPolicy can be specified only for cdrom, floppy and USB hostdevs for now. This particular functionality calls for potentional problems IMHO in case it is not properly documented. Also, 'requisite' for disks would mean that you have to hot-unplug the disk if it needs to be removed due to migration because otherwise qemu wouldn't start on the destination, or would it? Yeah, the hot-unplugging disk is necessary in the case of 'requisite' if diskchains checking failed during migration. The more I'm staring into the code, the more I feel like this is not a good idea and should be managed from upper application if this behavior is required for disks. I am not sure it is upper application's work. libvirt collected diskchain data for each disk at guest bootup already so it is easier to remove or hot-unplug a disk if backingchain is broken from libvirt point of view. upper application know nothing about backingchain for guest disks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/4] storage: report error rather than warning if backing files doesn't exist
On 07/15/2013 09:31 PM, Martin Kletzander wrote: s/doesn't/don't/ in $SUBJ On 07/02/2013 11:35 AM, Guannan Ren wrote: If one of backing files for disk source doesn't exist, the guest will not be able to find and use the disk even though the disk still exists in guest xml definition. So reporting an error make more sense. Adding virFileAccessibleAs() to check if the backing file described in disk meta exist in real path. If not, report error. the uid and gid s/exist/exists/; s/the/The/ arguments don't have so much meannings for F_OK, so give 0 for them. s/meannings/meaning/ --- src/util/virstoragefile.c | 23 +++ tests/virstoragetest.c| 16 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 27aa4fe..cb61e5b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -580,6 +580,13 @@ virFindBackingFile(const char *start, bool start_is_dir, const char *path, goto cleanup; } +if (virFileAccessibleAs(combined, F_OK, 0, 0) 0) { +virReportSystemError(errno, + _(Backing file '%s' does not exist), + combined); +goto cleanup; +} + Pre-existing, but this nad other errors will be shadowed by the error in qemuDomainBlockCommit. The existence of the file is already checked by: if (!(*canonical = canonicalize_file_name(combined))) { this ^^ line, so no need to call virFileAccessibleAs(). Calling virFileAccessibleAs() here is for more clear error message. virReportSystemError(errno, _(Can't canonicalize path '%s'), path); @@ -870,14 +877,10 @@ virStorageFileGetMetadataInternal(const char *path, !!directory, backing, meta-directory, meta-backingStore) 0) { -/* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ -meta-backingStoreIsFile = false; -backingFormat = VIR_STORAGE_FILE_NONE; -VIR_WARN(Backing file '%s' of image '%s' is missing., - meta-backingStoreRaw, path); - +VIR_ERROR(_(Backing file '%s' of image '%s' is missing.), + meta-backingStoreRaw, path); +VIR_FREE(backing); +goto cleanup; And then you report one more error here, even though the function may fail because of something else. I'd guess that's why it was a warning. No idea why it skipped the errors, though. The errors should be fixed, but this patch just adds more confusion to the error reporting. no, VIR_ERROR just adds one more error log item, the actual raised error is still there not being shadowed. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 16/32] Convert 'int i' to 'size_t i' in src/qemu files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_agent.c| 9 ++-- src/qemu/qemu_capabilities.c | 7 +-- src/qemu/qemu_cgroup.c | 12 ++--- src/qemu/qemu_command.c | 104 +++-- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_domain.c | 34 ++-- src/qemu/qemu_driver.c | 120 +++ src/qemu/qemu_hostdev.c | 26 +- src/qemu/qemu_hotplug.c | 54 ++- src/qemu/qemu_migration.c| 25 - src/qemu/qemu_monitor.c | 13 ++--- src/qemu/qemu_monitor_json.c | 31 ++- src/qemu/qemu_monitor_text.c | 6 +-- src/qemu/qemu_process.c | 48 - 14 files changed, 259 insertions(+), 232 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1b9ba41..44f880d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12227,9 +12232,10 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = qemuMonitorTransaction(priv-mon, actions); virJSONValueFree(actions); if (ret 0) { +int idx = i; here I think ssize idx is better /* Transaction failed; undo the changes to vm. */ bool need_unlink = !(flags VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT); -while (--i = 0) { +while (--idx = 0) { virDomainDiskDefPtr persistDisk = NULL; if (snap-def-disks[i].snapshot == @@ -12247,6 +12253,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, snap-def-dom-disks[i], vm-def-disks[i], persistDisk, + useless blank line here. need_unlink); } } ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 20/32] Convert 'int i' to 'size_t i' in src/nwfilter/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/nwfilter/nwfilter_driver.c| 5 +++-- src/nwfilter/nwfilter_ebiptables_driver.c | 4 ++-- src/nwfilter/nwfilter_gentech_driver.c| 21 +++-- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 0fbc940..57f1f54 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -441,7 +441,8 @@ nwfilterClose(virConnectPtr conn) { static int nwfilterConnectNumOfNWFilters(virConnectPtr conn) { virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData; -int i, n; +size_t i; +int n; if (virConnectNumOfNWFiltersEnsureACL(conn) 0) return -1; @@ -503,7 +504,7 @@ nwfilterConnectListAllNWFilters(virConnectPtr conn, int nfilters = 0; virNWFilterPtr filter = NULL; virNWFilterObjPtr obj = NULL; -int i; +size_t i; int ret = -1; virCheckFlags(0, -1); missed i in nwfilterConnectListNWFilters() diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 9a54de4..ee676b7 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -3705,7 +3705,7 @@ ebiptablesApplyNewRules(const char *ifname, int nruleInstances, void **_inst) { -int i, j; +size_t i, j; int cli_status; ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4078,7 +4078,7 @@ ebiptablesRemoveRules(const char *ifname ATTRIBUTE_UNUSED, { int rc = 0; int cli_status; -int i; +size_t i; virBuffer buf = VIR_BUFFER_INITIALIZER; ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst; missed i in _ebtablesRemoveSubChains() ebtablesRenameTmpSubAndRootChains() ebtablesCreateTmpRootAndSubChains() -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 21/32] Convert 'int i' to 'size_t i' in src/node_device/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/node_device/node_device_driver.c | 6 +++--- src/node_device/node_device_hal.c| 9 ++--- src/nodeinfo.c | 24 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 1f7e0fd..c7b15e1 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -131,7 +131,7 @@ nodeNumOfDevices(virConnectPtr conn, { virNodeDeviceDriverStatePtr driver = conn-nodeDevicePrivateData; int ndevs = 0; -unsigned int i; +size_t i; if (virNodeNumOfDevicesEnsureACL(conn) 0) return -1; @@ -161,7 +161,7 @@ nodeListDevices(virConnectPtr conn, { virNodeDeviceDriverStatePtr driver = conn-nodeDevicePrivateData; int ndevs = 0; -unsigned int i; +size_t i; if (virNodeListDevicesEnsureACL(conn) 0) return -1; @@ -249,7 +249,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, const char *wwpn, unsigned int flags) { -unsigned int i; +size_t i; virNodeDeviceDriverStatePtr driver = conn-nodeDevicePrivateData; virNodeDeviceObjListPtr devs = driver-devs; virNodeDevCapsDefPtr cap = NULL; diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 794f923..d94767c 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -408,7 +408,8 @@ gather_capabilities(LibHalContext *ctx, const char *udi, char *bus_name = NULL; virNodeDevCapsDefPtr caps = NULL; char **hal_cap_names = NULL; -int rv, i; +int rv; +size_t i; if (STREQ(udi, /org/freedesktop/Hal/devices/computer)) { rv = gather_capability(ctx, udi, system, caps); @@ -632,7 +633,8 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED, { LibHalContext *hal_ctx = NULL; char **udi = NULL; -int num_devs, i; +int num_devs; +size_t i; int ret = -1; DBusConnection *sysbus; DBusError err; @@ -750,7 +752,8 @@ nodeStateReload(void) { DBusError err; char **udi = NULL; -int num_devs, i; +int num_devs; +size_t i; LibHalContext *hal_ctx; VIR_INFO(Reloading HAL device state); diff --git a/src/nodeinfo.c b/src/nodeinfo.c index a50f892..d48c7ed 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -157,7 +157,7 @@ virNodeCountThreadSiblings(const char *dir, unsigned int cpu) char *path; FILE *pathfp; char str[1024]; -int i; +size_t i; if (virAsprintf(path, %s/cpu%u/topology/thread_siblings, dir, cpu) 0) { @@ -221,7 +221,7 @@ virNodeParseSocket(const char *dir, unsigned int cpu) static int CPU_COUNT(cpu_set_t *set) { -int i, count = 0; +size_t i, count = 0; for (i = 0; i CPU_SETSIZE; i++) if (CPU_ISSET(i, set)) @@ -251,7 +251,7 @@ virNodeParseNode(const char *node, int sock; cpu_set_t *core_maps = NULL; int core; -int i; +size_t i; int siblings; unsigned int cpu; int online; @@ -635,7 +635,7 @@ int linuxNodeGetCPUStats(FILE *procstat, char *buf = line; if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */ -int i; +size_t i; if (sscanf(buf, %*s %llu %llu %llu %llu %llu // user ~ iowait @@ -709,7 +709,7 @@ int linuxNodeGetMemoryStats(FILE *meminfo, int *nparams) { int ret = -1; -int i = 0, j = 0, k = 0; +size_t i = 0, j = 0, k = 0; int found = 0; int nr_param; char line[1024]; @@ -1041,7 +1041,7 @@ nodeGetCPUCount(void) * will be consecutive. */ char *cpupath = NULL; -int i = 0; +size_t i = 0; linuxParseCPUmax can return -1 to i. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 25/32] Convert 'int i' to 'size_t i' in src/network/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/network/bridge_driver.c | 281 +++- 1 file changed, 144 insertions(+), 137 deletions(-) @@ -3757,7 +3762,7 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { unsigned int num_virt_fns = 0; char **vfname = NULL; virPCIDeviceAddressPtr *virt_fns; -int ret = -1, ii = 0; +int ret = -1, i = 0; i is supposed to be type size_t here, isn't? if ((virNetDevGetVirtualFunctions(netdef-forward.pfs-dev, vfname, virt_fns, num_virt_fns)) 0) { @@ -3781,14 +3786,14 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { netdef-forward.nifs = num_virt_fns; -for (ii = 0; ii netdef-forward.nifs; ii++) { +for (i = 0; i netdef-forward.nifs; i++) { if ((netdef-forward.type == VIR_NETWORK_FORWARD_BRIDGE) || (netdef-forward.type == VIR_NETWORK_FORWARD_PRIVATE) || (netdef-forward.type == VIR_NETWORK_FORWARD_VEPA) || (netdef-forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH)) { -netdef-forward.ifs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; -if (vfname[ii]) { -if (VIR_STRDUP(netdef-forward.ifs[ii].device.dev, vfname[ii]) 0) +netdef-forward.ifs[i].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; +if (vfname[i]) { +if (VIR_STRDUP(netdef-forward.ifs[i].device.dev, vfname[i]) 0) goto finish; } else { virReportError(VIR_ERR_INTERNAL_ERROR, %s, @@ -3798,19 +3803,19 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { } else if (netdef-forward.type == VIR_NETWORK_FORWARD_HOSTDEV) { /* VF's are always PCI devices */ -netdef-forward.ifs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI; -netdef-forward.ifs[ii].device.pci.domain = virt_fns[ii]-domain; -netdef-forward.ifs[ii].device.pci.bus = virt_fns[ii]-bus; -netdef-forward.ifs[ii].device.pci.slot = virt_fns[ii]-slot; -netdef-forward.ifs[ii].device.pci.function = virt_fns[ii]-function; +netdef-forward.ifs[i].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI; +netdef-forward.ifs[i].device.pci.domain = virt_fns[i]-domain; +netdef-forward.ifs[i].device.pci.bus = virt_fns[i]-bus; +netdef-forward.ifs[i].device.pci.slot = virt_fns[i]-slot; +netdef-forward.ifs[i].device.pci.function = virt_fns[i]-function; } } ret = 0; finish: -for (ii = 0; ii num_virt_fns; ii++) { -VIR_FREE(vfname[ii]); -VIR_FREE(virt_fns[ii]); +for (i = 0; i num_virt_fns; i++) { +VIR_FREE(vfname[i]); +VIR_FREE(virt_fns[i]); } VIR_FREE(vfname); VIR_FREE(virt_fns); missed networkConnectNumOfNetworks() networkConnectListNetworks() networkConnectNumOfDefinedNetworks() networkConnectListDefinedNetworks() Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 30/32] Convert 'int i' to 'size_t i' in python/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- python/libvirt-lxc-override.c | 2 +- python/libvirt-override.c | 111 ++ 2 files changed, 69 insertions(+), 44 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 5c5586d..fcdc91b 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c missed virConnectCredCallbackWrapper() -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/32] Convert 'int i' to 'size_t i' in src/conf/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/conf/capabilities.c| 32 ++-- src/conf/cpu_conf.c| 18 +-- src/conf/domain_audit.c| 4 +- src/conf/domain_conf.c | 266 --- src/conf/domain_conf.h | 12 +- src/conf/domain_event.c| 24 +-- src/conf/domain_nwfilter.c | 2 +- src/conf/interface_conf.c | 73 + src/conf/netdev_vlan_conf.c| 19 +-- src/conf/network_conf.c| 319 +++-- src/conf/node_device_conf.c| 27 ++-- src/conf/nwfilter_conf.c | 43 ++--- src/conf/nwfilter_params.c | 26 +-- src/conf/snapshot_conf.c | 21 +-- src/conf/storage_conf.c| 42 ++--- src/conf/storage_encryption_conf.c | 3 +- 16 files changed, 482 insertions(+), 449 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index b2be99a..068b09d 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2361,7 +2362,7 @@ virNWFilterRuleParse(xmlNodePtr node) while (cur != NULL) { if (cur-type == XML_ELEMENT_NODE) { -int i = 0; +size_t i = 0; while (1) { if (found) i = found_i; If we should also convert int found_i = 0 to size_t found_i = 0 in virNWFilterRuleParse() diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index 48b89b7..a20b7b0 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c In nwfilter_params.c, missed i in virNWFilterVarValueFree() and virNWFilterVarValueCopy() Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/32] Convert 'int i' to 'size_t i' in src/util/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/util/virbitmap.c | 13 +++-- src/util/vircgroup.c | 32 src/util/vircommand.c| 10 +- src/util/virdnsmasq.c| 16 src/util/virebtables.c | 8 src/util/vireventpoll.c | 28 ++-- src/util/virhook.c | 3 ++- src/util/virjson.c | 12 ++-- src/util/virkeycode.c| 4 ++-- src/util/virlog.c| 30 +++--- src/util/virlog.h| 2 +- src/util/virmacaddr.c| 2 +- src/util/virnetdevmacvlan.c | 2 +- src/util/virnetdevopenvswitch.c | 2 +- src/util/virnetdevtap.c | 4 ++-- src/util/virnetdevvportprofile.c | 2 +- src/util/virnetlink.c| 14 -- src/util/virnuma.c | 13 +++-- src/util/virpci.c| 14 +++--- src/util/virportallocator.c | 8 src/util/virprocess.c| 7 --- src/util/virscsi.c | 6 +++--- src/util/virsocketaddr.c | 28 ++-- src/util/virstoragefile.c| 7 --- src/util/virstring.c | 3 ++- src/util/virsysinfo.c| 6 +++--- src/util/virthreadwin32.c| 6 +++--- src/util/virthreadwin32.h| 2 +- src/util/virtypedparam.c | 10 +- src/util/viruri.c| 2 +- src/util/virusb.c| 6 +++--- src/util/virutil.c | 29 - src/util/viruuid.c | 7 --- 33 files changed, 175 insertions(+), 163 deletions(-) ... diff --git a/src/util/virpci.c b/src/util/virpci.c index 54f7715..a3353cc 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1672,7 +1672,7 @@ static void virPCIDeviceListDispose(void *obj) { virPCIDeviceListPtr list = obj; -int i; +size_t i; for (i = 0; i list-count; i++) { virPCIDeviceFree(list-devs[i]); @@ -1780,7 +1780,7 @@ virPCIDeviceListDel(virPCIDeviceListPtr list, int virPCIDeviceListFindIndex(virPCIDeviceListPtr list, virPCIDevicePtr dev) { -int i; +size_t i; for (i = 0; i list-count; i++) if (list-devs[i]-domain == dev-domain @@ -1799,7 +1799,7 @@ virPCIDeviceListFindByIDs(virPCIDeviceListPtr list, unsigned int slot, unsigned int function) { -int i; +size_t i; for (i = 0; i list-count; i++) { if (list-devs[i]-domain == domain @@ -1815,10 +1815,10 @@ virPCIDeviceListFindByIDs(virPCIDeviceListPtr list, virPCIDevicePtr virPCIDeviceListFind(virPCIDeviceListPtr list, virPCIDevicePtr dev) { -int i; +int idx; -if ((i = virPCIDeviceListFindIndex(list, dev)) = 0) -return list-devs[i]; +if ((idx = virPCIDeviceListFindIndex(list, dev)) = 0) +return list-devs[idx]; else return NULL; } @@ -2416,7 +2416,7 @@ virPCIGetVirtualFunctions(const char *sysfs_path, unsigned int *num_virtual_functions) { int ret = -1; -int i; +size_t i; DIR *dir = NULL; struct dirent *entry = NULL; char *device_link = NULL; diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 0757966..6730d00 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -98,7 +98,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, unsigned short *port) { int ret = -1; -int i; +size_t i; int fd = -1; *port = 0; @@ -112,7 +112,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, if (virBitmapGetBit(pa-bitmap, i - pa-start, used) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _(Failed to query port %d), i); + _(Failed to query port %zu), i); goto cleanup; } @@ -138,7 +138,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, if (bind(fd, (struct sockaddr*)addr, sizeof(addr)) 0) { if (errno != EADDRINUSE) { virReportSystemError(errno, - _(Unable to bind to port %d), i); + _(Unable to bind to port %zu), i); goto cleanup; } /* In use, try next */ @@ -148,7 +148,7 @@ int
Re: [libvirt] [PATCH 05/32] Convert 'int i' to 'size_t i' in tools/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tools/virsh-domain-monitor.c | 17 + tools/virsh-domain.c | 41 +++-- tools/virsh-host.c | 8 tools/virsh-interface.c | 6 +++--- tools/virsh-network.c| 6 +++--- tools/virsh-nodedev.c| 8 tools/virsh-nwfilter.c | 6 +++--- tools/virsh-pool.c | 7 --- tools/virsh-secret.c | 6 +++--- tools/virsh-snapshot.c | 12 ++-- tools/virsh-volume.c | 6 +++--- tools/virsh.c| 15 --- 12 files changed, 73 insertions(+), 65 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5257416..be650c2 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c In cmdUndefine(), there are two variables of int type needing conversion. int vol_i int tok_i Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/32] Convert 'int i' to 'size_t i' in src/{xen, xenapi, xenxs} files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrangeberra...@redhat.com --- src/xen/block_stats.c | 4 ++-- src/xen/xen_driver.c | 4 ++-- src/xen/xen_hypervisor.c | 14 -- src/xen/xen_inotify.c | 4 ++-- src/xen/xend_internal.c| 12 +++- src/xen/xm_internal.c | 5 +++-- src/xen/xs_internal.c | 27 +-- src/xenapi/xenapi_driver.c | 30 ++ src/xenapi/xenapi_utils.c | 14 +++--- src/xenxs/xen_sxpr.c | 10 +- src/xenxs/xen_xm.c | 6 +++--- 11 files changed, 74 insertions(+), 56 deletions(-) --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1267,7 +1267,7 @@ xenHypervisorSetSchedulerParameters(virConnectPtr conn, virTypedParameterPtr params, int nparams) { -int i; +size_t i; unsigned int val; xenUnifiedPrivatePtr priv = conn-privateData; char buf[256]; @@ -2043,7 +2043,7 @@ xenHypervisorBuildCapabilities(virConnectPtr conn, virArch hostarch, int nr_guest_archs) { virCapsPtr caps; -int i; +size_t i; int hv_major = hv_versions.hv 16; int hv_minor = hv_versions.hv 0x; @@ -2223,7 +2223,7 @@ static virCapsPtr xenHypervisorMakeCapabilitiesSunOS(virConnectPtr conn) { struct guest_arch guest_arches[32]; -int i = 0; +size_t i = 0; virCapsPtr caps = NULL; int pae, longmode; const char *hvm; @@ -2293,7 +2293,7 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, char line[1024], *str, *token; regmatch_t subs[4]; char *saveptr = NULL; -int i; +size_t i; char hvm_type[4] = ; /* vmx or svm (or if not in CPU). */ int host_pae = 0; @@ -2846,7 +2846,8 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, int maxCells) { xen_op_v2_sys op_sys; -int i, j, ret; +size_t i, j; +int ret; xenUnifiedPrivatePtr priv = conn-privateData; if (priv-nbNodeCells 0) { @@ -2976,7 +2977,8 @@ xenHypervisorGetVcpus(virConnectPtr conn, int ret; xenUnifiedPrivatePtr priv = conn-privateData; virVcpuInfoPtr ipt; -int nbinfo, i; +int nbinfo; +size_t i; if (sizeof(cpumap_t) 7) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, Missed int i in xenHypervisorLookupDomainByUUID() Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/32] Convert 'int i' to 'size_t i' in src/storage/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/storage/storage_backend.c | 8 src/storage/storage_backend_disk.c| 6 +++--- src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_iscsi.c | 2 +- src/storage/storage_backend_logical.c | 10 ++ src/storage/storage_backend_rbd.c | 4 ++-- src/storage/storage_driver.c | 19 +++ 7 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 9a3bcf8..fd42a99 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -640,7 +640,7 @@ virStorageBackendCreateQemuImgOpts(char **opts, { virBuffer buf = VIR_BUFFER_INITIALIZER; bool b; -int i; +size_t i; if (backingType) virBufferAsprintf(buf, backing_fmt=%s,, backingType); @@ -1077,7 +1077,7 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageBackendPtr virStorageBackendForType(int type) { -unsigned int i; +size_t i; for (i = 0; backends[i]; i++) if (backends[i]-type == type) return backends[i]; @@ -1372,7 +1372,7 @@ int virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target, int fd) { -int i; +size_t i; off_t start; unsigned char buffer[1024]; ssize_t bytes; @@ -1674,7 +1674,7 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, FILE *fp = NULL; char **v; int ret = -1; -int i; +size_t i; if (n_columns == 0) return -1; missed int i, j in virStorageBackendRunProgRegex() diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index d15b3d4..d90d164 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -67,7 +67,7 @@ static void storageDriverUnlock(virStorageDriverStatePtr driver) static void storageDriverAutostart(virStorageDriverStatePtr driver) { -unsigned int i; +size_t i; for (i = 0; i driver-pools.count; i++) { virStoragePoolObjPtr pool = driver-pools.objs[i]; @@ -341,7 +341,8 @@ storageClose(virConnectPtr conn) { static int storageConnectNumOfStoragePools(virConnectPtr conn) { virStorageDriverStatePtr driver = conn-storagePrivateData; -unsigned int i, nactive = 0; +size_t i; +int nactive = 0; if (virConnectNumOfStoragePoolsEnsureACL(conn) 0) return -1; @@ -398,7 +399,8 @@ storageConnectListStoragePools(virConnectPtr conn, static int storageConnectNumOfDefinedStoragePools(virConnectPtr conn) { virStorageDriverStatePtr driver = conn-storagePrivateData; -unsigned int i, nactive = 0; +size_t i; +int nactive = 0; if (virConnectNumOfDefinedStoragePoolsEnsureACL(conn) 0) return -1; @@ -1199,7 +1201,8 @@ storagePoolListVolumes(virStoragePoolPtr obj, int maxnames) { virStorageDriverStatePtr driver = obj-conn-storagePrivateData; virStoragePoolObjPtr pool; -int i, n = 0; +size_t i; +int n = 0; memset(names, 0, maxnames * sizeof(*names)); @@ -1249,7 +1252,7 @@ storagePoolListAllVolumes(virStoragePoolPtr pool, unsigned int flags) { virStorageDriverStatePtr driver = pool-conn-storagePrivateData; virStoragePoolObjPtr obj; -int i; +size_t i; virStorageVolPtr *tmp_vols = NULL; virStorageVolPtr vol = NULL; int nvols = 0; @@ -1369,7 +1372,7 @@ static virStorageVolPtr storageVolLookupByKey(virConnectPtr conn, const char *key) { virStorageDriverStatePtr driver = conn-storagePrivateData; -unsigned int i; +size_t i; virStorageVolPtr ret = NULL; storageDriverLock(driver); @@ -1408,7 +1411,7 @@ static virStorageVolPtr storageVolLookupByPath(virConnectPtr conn, const char *path) { virStorageDriverStatePtr driver = conn-storagePrivateData; -unsigned int i; +size_t i; virStorageVolPtr ret = NULL; char *cleanpath; @@ -2282,7 +2285,7 @@ storageVolDelete(virStorageVolPtr obj, virStoragePoolObjPtr pool; virStorageBackendPtr backend; virStorageVolDefPtr vol = NULL; -unsigned int i; +size_t i; int ret = -1; storageDriverLock(driver); missed storageConnectListStoragePools() storageConnectListDefinedStoragePools() storagePoolNumOfVolumes() Guannan -- libvir-list mailing list libvir-list@redhat.com
Re: [libvirt] [PATCH 09/32] Convert 'int i' to 'size_t i' in src/vbox/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/vbox/vbox_XPCOMCGlue.c | 6 +- src/vbox/vbox_tmpl.c | 159 +++-- 2 files changed, 85 insertions(+), 80 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/32] Convert 'int i' to 'size_t i' in src/test/ files
On 07/08/2013 10:21 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Convert the type of loop iterators named 'i', 'j', k', 'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or 'unsigned int', also santizing 'ii', 'jj', 'kk' to use the normal 'i', 'j', 'k' naming Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/test/test_driver.c | 50 ++ 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 80a84d5..8e8e3f2 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -167,7 +167,7 @@ testBuildCapabilities(virConnectPtr conn) { virCapsPtr caps; virCapsGuestPtr guest; const char *const guest_types[] = { hvm, xen }; -int i; +size_t i; if ((caps = virCapabilitiesNew(VIR_ARCH_I686, 0, 0)) == NULL) goto no_memory; @@ -331,7 +331,8 @@ static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info); static char * testDomainGenerateIfname(virDomainDefPtr domdef) { int maxif = 1024; -int ifctr, i; +int ifctr; +size_t i; for (ifctr = 0; ifctr maxif; ++ifctr) { char *ifname; @@ -363,7 +364,7 @@ testDomainGenerateIfname(virDomainDefPtr domdef) { static int testDomainGenerateIfnames(virDomainDefPtr domdef) { -int i = 0; +size_t i = 0; for (i = 0; i domdef-nnets; i++) { char *ifname; @@ -391,7 +392,7 @@ testDomainUpdateVCPU(virConnectPtr conn ATTRIBUTE_UNUSED, testDomainObjPrivatePtr privdata = dom-privateData; virVcpuInfoPtr info = privdata-vcpu_infos[vcpu]; unsigned char *cpumap = VIR_GET_CPUMAP(privdata-cpumaps, maplen, vcpu); -int j; +size_t j; bool cpu; memset(info, 0, sizeof(virVcpuInfo)); @@ -440,7 +441,8 @@ testDomainUpdateVCPUs(virConnectPtr conn, { testConnPtr privconn = conn-privateData; testDomainObjPrivatePtr privdata = dom-privateData; -int i, ret = -1; +size_t i; +int ret = -1; int cpumaplen, maxcpu; maxcpu = VIR_NODEINFO_MAXCPUS(privconn-nodeInfo); @@ -705,7 +707,8 @@ static int testOpenVolumesForPool(xmlDocPtr xml, virStoragePoolObjPtr pool, int poolidx) { char *vol_xpath; -int i, ret, func_ret = -1; +size_t i; +int ret, func_ret = -1; xmlNodePtr *vols = NULL; virStorageVolDefPtr def = NULL; @@ -778,7 +781,8 @@ error: static int testOpenFromFile(virConnectPtr conn, const char *file) { -int i, ret; +size_t i; +int ret; long l; char *str; xmlDocPtr xml = NULL; @@ -2303,7 +2307,8 @@ static int testDomainGetVcpus(virDomainPtr domain, testConnPtr privconn = domain-conn-privateData; testDomainObjPrivatePtr privdomdata; virDomainObjPtr privdom; -int i, v, maxcpu, hostcpus; +size_t i; +int v, maxcpu, hostcpus; int ret = -1; struct timeval tv; unsigned long long statbase; @@ -2391,7 +2396,8 @@ static int testDomainPinVcpu(virDomainPtr domain, testDomainObjPrivatePtr privdomdata; virDomainObjPtr privdom; unsigned char *privcpumap; -int i, maxcpu, hostcpus, privmaplen; +size_t i; +int maxcpu, hostcpus, privmaplen; int ret = -1; testDriverLock(privconn); @@ -2548,7 +2554,7 @@ static int testNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freemems, int startCell, int maxCells) { testConnPtr privconn = conn-privateData; -int i, j; +size_t i, j; int ret = -1; testDriverLock(privconn); @@ -2879,7 +2885,8 @@ static int testDomainInterfaceStats(virDomainPtr domain, virDomainObjPtr privdom; struct timeval tv; unsigned long long statbase; -int i, found = 0, ret = -1; +size_t i; +int found = 0, ret = -1; testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn-domains, @@ -3467,7 +3474,8 @@ static int testInterfaceClose(virConnectPtr conn) static int testConnectNumOfInterfaces(virConnectPtr conn) { testConnPtr privconn = conn-privateData; -int i, count = 0; +size_t i; +int count = 0; testDriverLock(privconn); for (i = 0; (i privconn-ifaces.count); i++) { @@ -3512,7 +3520,8 @@ error: static int testConnectNumOfDefinedInterfaces(virConnectPtr conn) { testConnPtr privconn = conn-privateData; -int i, count = 0; +size_t i; +int count = 0; testDriverLock(privconn); for (i = 0; i privconn-ifaces.count; i++) { @@ -4612,7 +4621,8 @@ testStoragePoolListVolumes(virStoragePoolPtr pool, int maxnames) { testConnPtr privconn = pool-conn-privateData; virStoragePoolObjPtr privpool; -int i = 0, n = 0; +
[libvirt] [PATCH v4 1/4] conf: add startupPolicy attribute for harddisk
Add startupPolicy attribute policy for harddisk with type file, block and dir. The network type disk is still not supported. --- docs/schemas/domaincommon.rng | 6 ++ src/conf/domain_conf.c| 20 +--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index cf82878..483f068 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1093,6 +1093,9 @@ ref name=absFilePath/ /attribute optional + ref name=startupPolicy/ +/optional +optional ref name='devSeclabel'/ /optional /element @@ -1110,6 +1113,9 @@ attribute name=dir ref name=absFilePath/ /attribute +optional + ref name=startupPolicy/ +/optional empty/ /element /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 011de71..1040b40 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4739,7 +4739,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, switch (def-type) { case VIR_DOMAIN_DISK_TYPE_FILE: source = virXMLPropString(cur, file); -startupPolicy = virXMLPropString(cur, startupPolicy); break; case VIR_DOMAIN_DISK_TYPE_BLOCK: source = virXMLPropString(cur, dev); @@ -4828,7 +4827,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, case VIR_DOMAIN_DISK_TYPE_VOLUME: if (virDomainDiskSourcePoolDefParse(cur, def) 0) goto error; -startupPolicy = virXMLPropString(cur, startupPolicy); break; default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -4837,6 +4835,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } +startupPolicy = virXMLPropString(cur, startupPolicy); + /* People sometimes pass a bogus '' source path when they mean to omit the source element completely (e.g. CDROM without media). This is @@ -5410,11 +5410,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } -if (def-device != VIR_DOMAIN_DISK_DEVICE_CDROM -def-device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { +if (def-type == VIR_DOMAIN_DISK_TYPE_NETWORK) { virReportError(VIR_ERR_INVALID_ARG, - _(Setting disk %s is allowed only for - cdrom or floppy), + _(Setting disk %s is not allowed for + disk of network type), startupPolicy); goto error; } @@ -13872,6 +13871,9 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, source dev='%s', def-src); +if (def-startupPolicy) +virBufferEscapeString(buf, startupPolicy='%s', + startupPolicy); if (def-nseclabels) { virBufferAddLit(buf, \n); virBufferAdjustIndent(buf, 8); @@ -13884,8 +13886,12 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, } break; case VIR_DOMAIN_DISK_TYPE_DIR: -virBufferEscapeString(buf, source dir='%s'/\n, +virBufferEscapeString(buf, source dir='%s', def-src); +if (def-startupPolicy) +virBufferEscapeString(buf, startupPolicy='%s', + startupPolicy); +virBufferAddLit(buf, /\n); break; case VIR_DOMAIN_DISK_TYPE_NETWORK: virBufferAsprintf(buf, source protocol='%s', -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 0/4] Add startupPolicy attribute support for hard disks
v3: https://www.redhat.com/archives/libvir-list/2013-June/thread.html v3 to v4: Rebase v2 to v3: Not only check disk source, startupPolicy should work if any backing file is missing. The commit 039a3283 break the limition of contiguous device boot orders, so I remove my previous patch about it. v1 to v2: Added relax schema for disk of block and dir type Removed original patch 3/5. The set of patches is trying to add 'startupPolicy' attribute support to the source element of hard disks. Policy levels are using the mandatory, requisite, optional levels as originally documented. For the 'optional' policy, there is a little difference from CDROM and Floppy which only drop its source path, for disks, if missing, the checking function will drop their definitions, because qemu doesn't allow missing source path for hard disk. Guannan Ren [PATCH v3 1/4] conf: add startupPolicy attribute for harddisk [PATCH v3 2/4] storage: report error rather than warning if backing files doesn't exist [PATCH v3 3/4] qemu: drop disk with 'optional' startupPolicy if it is not present [PATCH v3 4/4] security: restore DAC for every disk file in disk chain docs/schemas/domaincommon.rng | 6 ++ include/libvirt/libvirt.h.in | 1 + src/conf/domain_conf.c| 20 +--- src/qemu/qemu_domain.c| 119 +-- src/qemu/qemu_process.c | 7 --- src/security/security_dac.c | 15 ++- src/util/virstoragefile.c | 23 +++ tests/virstoragetest.c| 16 8 files changed, 130 insertions(+), 77 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 2/4] storage: report error rather than warning if backing files doesn't exist
If one of backing files for disk source doesn't exist, the guest will not be able to find and use the disk even though the disk still exists in guest xml definition. So reporting an error make more sense. Adding virFileAccessibleAs() to check if the backing file described in disk meta exist in real path. If not, report error. the uid and gid arguments don't have so much meannings for F_OK, so give 0 for them. --- src/util/virstoragefile.c | 23 +++ tests/virstoragetest.c| 16 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 27aa4fe..cb61e5b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -580,6 +580,13 @@ virFindBackingFile(const char *start, bool start_is_dir, const char *path, goto cleanup; } +if (virFileAccessibleAs(combined, F_OK, 0, 0) 0) { +virReportSystemError(errno, + _(Backing file '%s' does not exist), + combined); +goto cleanup; +} + if (!(*canonical = canonicalize_file_name(combined))) { virReportSystemError(errno, _(Can't canonicalize path '%s'), path); @@ -870,14 +877,10 @@ virStorageFileGetMetadataInternal(const char *path, !!directory, backing, meta-directory, meta-backingStore) 0) { -/* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ -meta-backingStoreIsFile = false; -backingFormat = VIR_STORAGE_FILE_NONE; -VIR_WARN(Backing file '%s' of image '%s' is missing., - meta-backingStoreRaw, path); - +VIR_ERROR(_(Backing file '%s' of image '%s' is missing.), + meta-backingStoreRaw, path); +VIR_FREE(backing); +goto cleanup; } } VIR_FREE(backing); @@ -1062,6 +1065,10 @@ virStorageFileGetMetadataRecurse(const char *path, const char *directory, uid, gid, allow_probe, cycle); +if (!ret-backingMeta) { +virStorageFileFreeMetadata(ret); +ret = NULL; +} } return ret; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index fef4b37..fb5da5c 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -483,10 +483,10 @@ mymain(void) /* Qcow2 file with missing backing file but specified type */ const testFileData chain9[] = { qcow2_bogus }; TEST_CHAIN(9, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2, - chain9, EXP_WARN, - chain9, ALLOW_PROBE | EXP_WARN, - chain9, EXP_WARN, - chain9, ALLOW_PROBE | EXP_WARN); + chain9, EXP_FAIL, + chain9, ALLOW_PROBE | EXP_FAIL, + chain9, EXP_FAIL, + chain9, ALLOW_PROBE | EXP_FAIL); /* Rewrite qcow2 to a missing backing file, without backing type */ virCommandFree(cmd); @@ -498,10 +498,10 @@ mymain(void) /* Qcow2 file with missing backing file and no specified type */ const testFileData chain10[] = { qcow2_bogus }; TEST_CHAIN(10, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2, - chain10, EXP_WARN, - chain10, ALLOW_PROBE | EXP_WARN, - chain10, EXP_WARN, - chain10, ALLOW_PROBE | EXP_WARN); + chain10, EXP_FAIL, + chain10, ALLOW_PROBE | EXP_FAIL, + chain10, EXP_FAIL, + chain10, ALLOW_PROBE | EXP_FAIL); /* Rewrite qcow2 to use an nbd: protocol as backend */ virCommandFree(cmd); -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 4/4] security: restore DAC for every disk file in disk chain
--- src/security/security_dac.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 0d6defc..577db8e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -335,6 +335,16 @@ err: return rc; } +static int +virSecurityDACRestoreDiskSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, + const char *path, + size_t depth ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ +return virSecurityDACRestoreSecurityFileLabel(path); +} + + static int virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, @@ -424,7 +434,10 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, } } -return virSecurityDACRestoreSecurityFileLabel(disk-src); +return virDomainDiskDefForeachPath(disk, + false, + virSecurityDACRestoreDiskSecurityFileLabel, + NULL); } -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 3/4] qemu: drop disk with 'optional' startupPolicy if it is not present
Go through disks of guest, if one disk doesn't exist or its backing chain is broken, with 'optional' startupPolicy, for CDROM and Floppy we only discard its source path definition in xml, for disks we drop it from disk list and free it. Originally, for these disks without startupPolicy attribute, libvirt ignore the presence checking, this patch make startupPolicy 'mandatory' as default. --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_domain.c | 119 ++- src/qemu/qemu_process.c | 7 --- 3 files changed, 74 insertions(+), 53 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b791125..5459eb4 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4699,6 +4699,7 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn, */ typedef enum { VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */ +VIR_DOMAIN_EVENT_HARDDISK_DROP_MISSING_ON_START = 1, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_DISK_CHANGE_LAST diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8d79066..d54d477 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2006,75 +2006,102 @@ cleanup: virObjectUnref(cfg); } +static int +qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ +char uuid[VIR_UUID_STRING_BUFLEN]; +virDomainEventPtr event = NULL; +virDomainDiskDefPtr del_disk = NULL; + +virUUIDFormat(vm-def-uuid, uuid); + +VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') + due to inaccessible source '%s', + disk-dst, vm-def-name, uuid, disk-src); + +if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM || +disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { + +event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, + disk-info.alias, + VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); + +VIR_FREE(disk-src); +} else { +event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL, + disk-info.alias, + VIR_DOMAIN_EVENT_HARDDISK_DROP_MISSING_ON_START); + +if (!(del_disk = virDomainDiskRemoveByName(vm-def, disk-src))) { +virReportError(VIR_ERR_INVALID_ARG, + _(no source device %s), disk-src); +return -1; +} +virDomainDiskDefFree(del_disk); +} + +if (event) +qemuDomainEventQueue(driver, event); + +return 0; +} + int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virDomainObjPtr vm, bool cold_boot) { int ret = -1; -int i; +size_t i; virDomainDiskDefPtr disk; -char uuid[VIR_UUID_STRING_BUFLEN]; -virDomainEventPtr event = NULL; -virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - -virUUIDFormat(vm-def-uuid, uuid); - -for (i = 0; i vm-def-ndisks; i++) { -disk = vm-def-disks[i]; +size_t count = vm-def-ndisks; +size_t nextdisk = 0; -if (!disk-startupPolicy || !disk-src) -continue; +for (i = 0; i count; i++) { +disk = vm-def-disks[nextdisk]; -if (virFileAccessibleAs(disk-src, F_OK, -cfg-user, -cfg-group) = 0) { -/* disk accessible */ +if (qemuDomainDetermineDiskChain(driver, disk, false) == 0) { +nextdisk++; continue; } switch ((enum virDomainStartupPolicy) disk-startupPolicy) { -case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL: -break; +case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL: +break; -case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); -goto cleanup; -break; - -case VIR_DOMAIN_STARTUP_POLICY_REQUISITE: -if (cold_boot) { -virReportSystemError(errno, - _(cannot access file '%s'), - disk-src); -goto cleanup; -} -break; +case VIR_DOMAIN_STARTUP_POLICY_REQUISITE: +if (cold_boot) { +goto error; +} +break; + +case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: +case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: +goto error; -case
Re: [libvirt] [PATCH] Crash of libvirtd by unprivileged user in virConnectListAllInterfaces
On 07/02/2013 10:14 PM, Eric Blake wrote: From: Daniel P. Berrange berra...@redhat.com On Thu, Jun 27, 2013 at 03:56:42PM +0100, Daniel P. Berrange wrote: Hi Security Team, I've discovered a way for an unprivileged user with a readonly connection to libvirtd, to crash the daemon. Ok, the final patch for this is issue will be the simpler variant that Eric suggested The embargo can be considered to be lifted on Monday July 1st, at 0900 UTC The following is the GIT change that DV or myself will apply to libvirt GIT master immediately before the 1.1.0 release: From 177b4165c531a4b3ba7f6ab6aa41dca9ceb0b8cf Mon Sep 17 00:00:00 2001 From: Daniel P. Berrange berra...@redhat.com Date: Fri, 28 Jun 2013 10:48:37 +0100 Subject: [PATCH] CVE-2013-2218: Fix crash listing network interfaces with filters The virConnectListAllInterfaces method has a double-free of the 'struct netcf_if' object when any of the filtering flags cause an interface to be skipped over. For example when running the command 'virsh iface-list --inactive' This is a regression introduced in release 1.0.6 by commit 7ac2c4fe624f30f2c8270116513fa2ddab07631f Author: Guannan Ren g...@redhat.com Date: Tue May 21 21:29:38 2013 +0800 interface: list all interfaces with flags == 0 Signed-off-by: Daniel P. Berrange berra...@redhat.com --- Posting as a courtesy FYI for anyone reading this list but who does not have access to the security list and doesn't want to crawl through git. This commit has been included in 1.1.0 and has been applied to all affected stable branches (just v1.0.6-maint). The rule in determining that a CVE was necessary is the escalation of privilege test - any time a read-only client can cause a denial-of-service against a more-privileged read-write client (by crashing libvirtd), there is an escalation. src/interface/interface_backend_netcf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index a995816..9aa673d 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -412,6 +412,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn, (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) (status NETCF_IFACE_INACTIVE { ncf_if_free(iface); +iface = NULL; continue; } Thanks for posting here. I have to say sorry about this bug I caused. This problem is caused by double-free 'iface' variable as Daniel pointed out when code path goes into cleanup label, there is a second ncf_if_free() for the variable if we didn't initialize it to NULL after the first ncf_if_free(). Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/5] snapshot: define new API virDomainSnapshotDeleteByName
This is to delete a snapshot object atomically, the API accepts argments: domain object, snapshot name and flags. When name is NULL, the current snapshot object will be deleted include/libvirt/libvirt.h.in: Declare virDomainSnapshotDeleteByName src/driver.h: (virDrvDomainSnapshotDeleteByName) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 4 +++ src/driver.h | 6 src/libvirt.c| 71 src/libvirt_public.syms | 5 4 files changed, 86 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b791125..e7e75c0 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4433,6 +4433,10 @@ typedef enum { int virDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags); +int virDomainSnapshotDeleteByName(virDomainPtr domain, + const char *name, + unsigned int flags); + int virDomainSnapshotRef(virDomainSnapshotPtr snapshot); int virDomainSnapshotFree(virDomainSnapshotPtr snapshot); diff --git a/src/driver.h b/src/driver.h index 31851cb..8651603 100644 --- a/src/driver.h +++ b/src/driver.h @@ -801,6 +801,11 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainSnapshotDeleteByName)(virDomainPtr domain, +const char *cmd, +unsigned int flags); + +typedef int (*virDrvDomainQemuMonitorCommand)(virDomainPtr domain, const char *cmd, char **result, @@ -1272,6 +1277,7 @@ struct _virDriver { virDrvDomainSnapshotHasMetadata domainSnapshotHasMetadata; virDrvDomainRevertToSnapshot domainRevertToSnapshot; virDrvDomainSnapshotDelete domainSnapshotDelete; +virDrvDomainSnapshotDeleteByName domainSnapshotDeleteByName; virDrvDomainQemuMonitorCommand domainQemuMonitorCommand; virDrvDomainQemuAttach domainQemuAttach; virDrvDomainQemuAgentCommand domainQemuAgentCommand; diff --git a/src/libvirt.c b/src/libvirt.c index bc1694a..aa04959 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20158,6 +20158,10 @@ error: * libvirt metadata to track snapshots, then this flag is silently * ignored. * + * Note that this command is inherently racy: another connection can + * delete the snapshot object between a call to virDomainSnapshotLookupByName() + * and this call. + * * Returns 0 if the selected snapshot(s) were successfully deleted, * -1 on error. */ @@ -20207,6 +20211,73 @@ error: } /** + * virDomainSnapshotDeleteByName: + * @domain: pointer to the domain object + * @name : snapshot name or NULL + * @flags: bitwise-OR of supported virDomainSnapshotDeleteFlags + * + * Delete the snapshot. + * + * If @flags is 0, then just this snapshot is deleted, and changes + * from this snapshot are automatically merged into children + * snapshots. If @flags includes VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, + * then this snapshot and any descendant snapshots are deleted. If + * @flags includes VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY, then any + * descendant snapshots are deleted, but this snapshot remains. These + * two flags are mutually exclusive. + * + * If @flags includes VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY, then + * any snapshot metadata tracked by libvirt is removed while keeping + * the snapshot contents intact; if a hypervisor does not require any + * libvirt metadata to track snapshots, then this flag is silently + * ignored. + * + * Returns 0 if the selected snapshot(s) were successfully deleted, + * -1 on error. + */ +int +virDomainSnapshotDeleteByName(virDomainPtr domain, + const char *name, + unsigned int flags) +{ +VIR_DOMAIN_DEBUG(domain, name=%s, flags=%x, name, flags); + +virResetLastError(); + +if (!VIR_IS_DOMAIN(domain)) { +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} + +if (domain-conn-flags VIR_CONNECT_RO) { +virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); +goto error; +} + +if ((flags VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) +(flags VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) { +virReportInvalidArg(flags, +_(children and children_only flags in %s are + mutually exclusive), +__FUNCTION__); +goto error; +} + +if (domain-conn-driver-domainSnapshotDelete) { +int ret = domain-conn-driver-domainSnapshotDeleteByName(domain, name, flags); +if (ret 0) +goto error; +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT,
[libvirt] [PATCH v2 2/5] snapshot: auto generate RPC calls for remoteDomainSnapshotDeleteByName
Let gendispatch.pl generate codes for both server side and client side. *src/remote/remote_driver.c: Add remoteDomainSnapshotDeleteByName into remote driver *src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE_BY_NAME and its argument structs *src/remote_protocol-structs: edit it to match --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +- src/remote_protocol-structs | 6 ++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7a0c1f6..fe7b836 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6721,6 +6721,7 @@ static virDriver remote_driver = { .domainSnapshotIsCurrent = remoteDomainSnapshotIsCurrent, /* 0.9.13 */ .domainSnapshotHasMetadata = remoteDomainSnapshotHasMetadata, /* 0.9.13 */ .domainSnapshotDelete = remoteDomainSnapshotDelete, /* 0.8.0 */ +.domainSnapshotDeleteByName = remoteDomainSnapshotDeleteByName, /* 1.1.1 */ .domainQemuMonitorCommand = remoteDomainQemuMonitorCommand, /* 0.8.3 */ .domainQemuAttach = remoteDomainQemuAttach, /* 0.9.4 */ .domainQemuAgentCommand = remoteDomainQemuAgentCommand, /* 0.10.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 2e9dc1d..aa3266b 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2475,6 +2475,12 @@ struct remote_domain_snapshot_delete_args { unsigned int flags; }; +struct remote_domain_snapshot_delete_by_name_args { +remote_nonnull_domain dom; +remote_string name; +unsigned int flags; +}; + struct remote_domain_open_console_args { remote_nonnull_domain dom; remote_string dev_name; @@ -4944,6 +4950,12 @@ enum remote_procedure { * @generate: none * @acl: domain:migrate */ -REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307 +REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307, + +/** + * @generate: both + * @acl: domain:snapshot + */ +REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE_BY_NAME = 308 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e38d24a..d9f5a68 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1904,6 +1904,11 @@ struct remote_domain_snapshot_delete_args { remote_nonnull_domain_snapshot snap; u_int flags; }; +struct remote_domain_snapshot_delete_by_name_args { +remote_nonnull_domain dom; +remote_string name; +u_int flags; +}; struct remote_domain_open_console_args { remote_nonnull_domain dom; remote_string dev_name; @@ -2601,4 +2606,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_PERFORM3_PARAMS = 305, REMOTE_PROC_DOMAIN_MIGRATE_FINISH3_PARAMS = 306, REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307, +REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE_BY_NAME = 308, }; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/5] virsh: use virDomainSnapshotDeleteByName in cmdSnapshotDelete
--- tools/virsh-snapshot.c | 65 ++ 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 7e75772..15bdcd7 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1966,37 +1966,82 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; virDomainSnapshotPtr snapshot = NULL; unsigned int flags = 0; +bool snapshotname = false; +bool children = vshCommandOptBool(cmd, children); +bool metadata = vshCommandOptBool(cmd, metadata); +bool children_only = vshCommandOptBool(cmd, children-only); +bool current = vshCommandOptBool(cmd, current); + +VSH_EXCLUSIVE_OPTIONS_VAR(children, children_only); + +if (vshCommandOptStringReq(ctl, cmd, snapshotname, name) 0) +goto cleanup; + +if (current name) { +snapshotname = true; +VSH_EXCLUSIVE_OPTIONS_VAR(current, snapshotname); +} dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) goto cleanup; +if (children) +flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN; +if (metadata) +flags |= VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY; +if (children_only) +flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY; + +if (current || name) { +if (virDomainSnapshotDeleteByName(dom, name, flags) == 0) +goto finished; +} else { +vshError(ctl, _(--snapshotname or --current is required)); +goto cleanup; +} + +if (last_error last_error-code == VIR_ERR_NO_SUPPORT) { +vshResetLibvirtError(); +goto fallback; +} + +goto error; + +fallback: if (vshLookupSnapshot(ctl, cmd, snapshotname, true, dom, snapshot, name) 0) goto cleanup; -if (vshCommandOptBool(cmd, children)) -flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN; -if (vshCommandOptBool(cmd, children-only)) -flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY; -if (vshCommandOptBool(cmd, metadata)) -flags |= VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY; - /* XXX If we wanted, we could emulate DELETE_CHILDREN_ONLY even on * older servers that reject the flag, by manually computing the * list of descendants. But that's a lot of code to maintain. */ -if (virDomainSnapshotDelete(snapshot, flags) == 0) { +if (virDomainSnapshotDelete(snapshot, flags) 0) +goto error; + +finished: +if (name) { if (flags VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) vshPrint(ctl, _(Domain snapshot %s children deleted\n), name); else vshPrint(ctl, _(Domain snapshot %s deleted\n), name); } else { -vshError(ctl, _(Failed to delete snapshot %s), name); -goto cleanup; +if (flags VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) +vshPrint(ctl, _(Domain current snapshot children deleted\n)); +else +vshPrint(ctl, _(Domain current snapshot deleted\n)); } ret = true; +error: +if (ret == false) { +if (name) +vshError(ctl, _(Failed to delete snapshot %s), name); +else +vshError(ctl, _(Failed to delete current snapshot)); +} + cleanup: if (snapshot) virDomainSnapshotFree(snapshot); -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/5]Atomic API to delete snapshot object
v1: https://www.redhat.com/archives/libvir-list/2013-June/msg00573.html v1-v2: Remove VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT flag (name == NULL) means deleting current snapshot object Rebase work Add a new snapshot API to delete snapshot object atomically int virDomainSnapshotDeleteByName(virDomainPtr domain, const char *name, unsigned int flags); The existing virDomainSnapshotDelete API accepts the snapshot object being deleted as an argument that would be not API atomic. Guannan Ren(5) [PATCH v2 1/5] snapshot: define new API virDomainSnapshotDeleteByName [PATCH v2 2/5] auto generate RPC calls for remoteDomainSnapshotDeleteByName [PATCH v2 3/5] qemu: implement SnapshotDeleteByName [PATCH v2 4/5] python: make auto-generated function name nicer [PATCH v2 5/5] virsh: use virDomainSnapshotDeleteByName in virsh include/libvirt/libvirt.h.in | 4 python/generator.py | 3 +++ src/driver.h | 6 ++ src/libvirt.c| 71 +++ src/libvirt_public.syms | 5 + src/qemu/qemu_driver.c | 103 +-- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +- src/remote_protocol-structs | 6 ++ tools/virsh-snapshot.c | 65 +++-- 10 files changed, 245 insertions(+), 33 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list