Re: [libvirt] [PATCH 3/3] storage: Split out volume upload/download as separate backend function
On 07/10/14 18:48, Eric Blake wrote: On 07/10/2014 08:30 AM, Peter Krempa wrote: For non-local storage drivers we can't expect to use the FDStream backend for up/downloading volumes. Split the code into a separate backend function so that we can add protocol specific code later. --- src/storage/storage_backend.c | 31 +++ src/storage/storage_backend.h | 30 ++ src/storage/storage_backend_disk.c| 2 ++ src/storage/storage_backend_fs.c | 7 ++ src/storage/storage_backend_iscsi.c | 2 ++ src/storage/storage_backend_logical.c | 2 ++ src/storage/storage_backend_mpath.c | 2 ++ src/storage/storage_backend_scsi.c| 2 ++ src/storage/storage_driver.c | 47 +++ 9 files changed, 93 insertions(+), 32 deletions(-) Without even reading this patch yet, this is more the approach I had in mind for 2/3. I've pushed this one according to Jan's ACK and will re-do 2/3 in the same fashion. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] doc: Document that snapshot name of block-backed disk isnt autogenerated
Libvirt generates external snapshot target file names for file backed storage but not for block backed storage. Document the limitation. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1032363 --- docs/formatsnapshot.html.in | 8 +--- tools/virsh.pod | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 1f9a6c6..4f7b7b2 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -164,9 +164,11 @@ attribute codetype/code giving the driver type (such as qcow2), of the new file created by the external snapshot of the new file. If codesource/code is not -given, a file name is generated that consists of the -existing file name with anything after the trailing dot -replaced by the snapshot name. Remember that with external +given and the disk is backed by a local image file (not +a block device or remote storage), a file name is +generated that consists of the existing file name +with anything after the trailing dot replaced by the +snapshot name. Remember that with external snapshots, the original file name becomes the read-only snapshot, and the new file name contains the read-write delta of all disk changes since the snapshot. diff --git a/tools/virsh.pod b/tools/virsh.pod index a5e8406..5da71c3 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3219,7 +3219,9 @@ The I--diskspec option can be used to control how I--disk-only and external checkpoints create external files. This option can occur multiple times, according to the number of disk elements in the domain xml. Each diskspec is in the -form Bdisk[,snapshot=type][,driver=type][,file=name]. To include a +form Bdisk[,snapshot=type][,driver=type][,file=name]. A Idiskspec +must be provided for disks backed by block devices as libvirt doesn't +auto-generate file names for those. To include a literal comma in Bdisk or in Bfile=name, escape it with a second comma. A literal I--diskspec must precede each Bdiskspec unless all three of Idomain, Iname, and Idescription are also present. -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] doc: Be more specific about semantics of _REUSE_EXT flag
Snapshots and block-copy have a flag that forces qemu to re-use existing file. Our docs weren't exactly clear on what the existing file should contain for this to actually work. Re-word the docs a bit to state that the file needs to be pre-created in the desired format and the backing chain metadata needs to be set prior to handing it over to qemu. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1084360 --- src/libvirt.c | 23 ++- tools/virsh.pod | 17 ++--- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 8684298..79bcdf1 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18394,10 +18394,12 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * destination files already exist as a non-empty regular file, the * snapshot is rejected to avoid losing contents of those files. * However, if @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, - * then the destination files must already exist and contain content - * identical to the source files (this allows a management app to - * pre-create files with relative backing file names, rather than the - * default of creating with absolute backing file names). + * then the destination files must be pre-created manually with + * the correct image format and metadata including backing store path + * (this allows a management app to pre-create files with relative backing + * file names, rather than the default of creating with absolute backing + * file names). Note that setting incorrect metadata in the pre-created + * image may lead to the VM being unable to start. * * Be aware that although libvirt prefers to report errors up front with * no other effect, some hypervisors have certain types of failures where @@ -19864,11 +19866,14 @@ virDomainBlockPull(virDomainPtr dom, const char *disk, * by adding VIR_DOMAIN_BLOCK_REBASE_COPY_RAW to force the copy to be raw * (does not make sense with the shallow flag unless the source is also raw), * or by using VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT to reuse an existing file - * with initial contents identical to the backing file of the source (this - * allows a management app to pre-create files with relative backing file - * names, rather than the default of absolute backing file names; as a - * security precaution, you should generally only use reuse_ext with the - * shallow flag and a non-raw destination file). + * which was pre-created with the correct format and metadata and sufficient + * size to hold the copy. In case the VIR_DOMAIN_BLOCK_REBASE_SHALLOW flag + * is used the pre-created file has to exhibit the same guest visible contents + * as the backing file of the original image. This allows a management app to + * pre-create files with relative backing file names, rather than the default + * of absolute backing file names; as a security precaution, you should + * generally only use reuse_ext with the shallow flag and a non-raw + * destination file. * * A copy job has two parts; in the first phase, the @bandwidth parameter * affects how fast the source is pulled into the destination, and the job diff --git a/tools/virsh.pod b/tools/virsh.pod index 5da71c3..1a2b01f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -876,10 +876,11 @@ flattens the entire chain; but if I--shallow is specified, the copy shares the backing chain. If I--reuse-external is specified, then Idest must exist and have -contents identical to the resulting backing file (that is, it must -start with contents matching the backing file Idisk if I--shallow -is used, otherwise it must start empty); this option is typically used -to set up a relative backing file name in the destination. +sufficient space to hold the copy. If I--shallow is used in +conjunction with I--reuse-external then the pre-created image must have +guest visible contents identical to guest visible contents of the backing +file of the original image. This may be used to modify the backing file +names on the destination. The format of the destination is determined by the first match in the following list: if I--raw is specified, it will be raw; if @@ -3172,7 +3173,8 @@ metadata again). If I--reuse-external is specified, and the snapshot XML requests an external snapshot with a destination of an existing file, then the -destination must exist, and is reused; otherwise, a snapshot is refused +destination must exist and be pre-created with correct format and +metadata. The file is then reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. If I--quiesce is specified, libvirt will try to use guest agent @@ -3233,8 +3235,9 @@ results in the following XML: If I--reuse-external is specified, and the domain XML or Idiskspec option requests an external snapshot with a destination of an existing -file, then the destination must exist, and is reused; otherwise, a -snapshot is refused to avoid losing contents of the
[libvirt] [PATCHv2 0/2] doc: Improve snapshot/blockjob docs
Peter Krempa (2): doc: Document that snapshot name of block-backed disk isnt autogenerated doc: Be more specific about semantics of _REUSE_EXT flag docs/formatsnapshot.html.in | 8 +--- src/libvirt.c | 23 ++- tools/virsh.pod | 21 + 3 files changed, 32 insertions(+), 20 deletions(-) -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] LXC: add support for --config in setmem command
In lxc, we could not use setmem command with --config options. This patch will add support for this. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v3: add max_balloon check for AFFECT_CONFIG v2: use virDomainSetMemoryFlagsEnsureACL remove redundant domain running check src/lxc/lxc_driver.c | 58 ++-- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3253211..f04b543 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -711,36 +711,64 @@ static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) return ret; } -static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) +static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, + unsigned int flags) { virDomainObjPtr vm; +virDomainDefPtr persistentDef = NULL; +virCapsPtr caps = NULL; int ret = -1; virLXCDomainObjPrivatePtr priv; +virLXCDriverPtr driver = dom-conn-privateData; +virLXCDriverConfigPtr cfg = NULL; +unsigned long oldmax = 0; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; +cfg = virLXCDriverGetConfig(driver); + priv = vm-privateData; -if (virDomainSetMemoryEnsureACL(dom-conn, vm-def) 0) +if (virDomainSetMemoryFlagsEnsureACL(dom-conn, vm-def, flags) 0) goto cleanup; -if (newmem vm-def-mem.max_balloon) { +if (!(caps = virLXCDriverGetCapabilities(driver, false))) +goto cleanup; + +if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags, +persistentDef) 0) +goto cleanup; + +if (flags VIR_DOMAIN_AFFECT_LIVE) +oldmax = vm-def-mem.max_balloon; +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!oldmax || oldmax persistentDef-mem.max_balloon) +oldmax = persistentDef-mem.max_balloon; +} + +if (newmem oldmax) { virReportError(VIR_ERR_INVALID_ARG, %s, _(Cannot set memory higher than max memory)); goto cleanup; } -if (!virDomainObjIsActive(vm)) { -virReportError(VIR_ERR_OPERATION_INVALID, - %s, _(Domain is not running)); -goto cleanup; +if (flags VIR_DOMAIN_AFFECT_LIVE) { +if (virCgroupSetMemory(priv-cgroup, newmem) 0) { +virReportError(VIR_ERR_OPERATION_FAILED, + %s, _(Failed to set memory for domain)); +goto cleanup; +} } -if (virCgroupSetMemory(priv-cgroup, newmem) 0) { -virReportError(VIR_ERR_OPERATION_FAILED, - %s, _(Failed to set memory for domain)); -goto cleanup; +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +sa_assert(persistentDef); +persistentDef-mem.cur_balloon = newmem; +if (virDomainSaveConfig(cfg-configDir, persistentDef) 0) +goto cleanup; } ret = 0; @@ -748,9 +776,16 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) cleanup: if (vm) virObjectUnlock(vm); +virObjectUnref(caps); +virObjectUnref(cfg); return ret; } +static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) +{ +return lxcDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_LIVE); +} + static int lxcDomainSetMemoryParameters(virDomainPtr dom, virTypedParameterPtr params, @@ -5697,6 +5732,7 @@ static virDriver lxcDriver = { .domainGetMaxMemory = lxcDomainGetMaxMemory, /* 0.7.2 */ .domainSetMaxMemory = lxcDomainSetMaxMemory, /* 0.7.2 */ .domainSetMemory = lxcDomainSetMemory, /* 0.7.2 */ +.domainSetMemoryFlags = lxcDomainSetMemoryFlags, /* 1.2.7 */ .domainSetMemoryParameters = lxcDomainSetMemoryParameters, /* 0.8.5 */ .domainGetMemoryParameters = lxcDomainGetMemoryParameters, /* 0.8.5 */ .domainSetBlkioParameters = lxcDomainSetBlkioParameters, /* 0.9.8 */ -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/3] conf: Always format seclabel's model
https://bugzilla.redhat.com/show_bug.cgi?id=1113860 We've always done that. Well, until 990e46c45. Point is, if we don't format model, we may lose a domain on libvirtd restart. If the seclabel is implicit however, we should skip it's formatting. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 34 +++--- .../qemuxml2argv-seclabel-dynamic-none.xml | 28 ++ tests/qemuxml2xmltest.c| 1 + 3 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b91ccf7..7b90903 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4614,8 +4614,23 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, /* For the model 'none' none of the following labels is going to be * present. Hence, return now. */ -if (STREQ_NULLABLE(def-model, none)) +if (STREQ_NULLABLE(def-model, none)) { +if (flags VIR_DOMAIN_XML_INACTIVE) { +/* Fix older configurations */ +def-type = VIR_DOMAIN_SECLABEL_NONE; +def-relabel = false; +} else { +if (def-type != VIR_DOMAIN_SECLABEL_NONE) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unsupported type='%s' to model 'none'), + virDomainSeclabelTypeToString(def-type)); +goto error; +} +/* combination of relabel='yes' and type='static' + * is checked a few lines above. */ +} return def; +} /* Only parse label, if using static labels, or * if the 'live' VM XML is requested @@ -14690,8 +14705,7 @@ virDomainEventActionDefFormat(virBufferPtr buf, static void virSecurityLabelDefFormat(virBufferPtr buf, - virSecurityLabelDefPtr def, - unsigned flags) + virSecurityLabelDefPtr def) { const char *sectype = virDomainSeclabelTypeToString(def-type); @@ -14701,19 +14715,17 @@ virSecurityLabelDefFormat(virBufferPtr buf, if (def-type == VIR_DOMAIN_SECLABEL_DEFAULT) return; -/* To avoid backward compatibility issues, suppress DAC labels that are - * automatically generated. +/* To avoid backward compatibility issues, suppress DAC and 'none' labels + * that are automatically generated. */ -if (STREQ_NULLABLE(def-model, dac) def-implicit) +if ((STREQ_NULLABLE(def-model, dac) || + STREQ_NULLABLE(def-model, none)) def-implicit) return; virBufferAsprintf(buf, seclabel type='%s', sectype); -/* When generating state XML do include the model */ -if (flags VIR_DOMAIN_XML_INTERNAL_STATUS || -STRNEQ_NULLABLE(def-model, none)) -virBufferEscapeString(buf, model='%s', def-model); +virBufferEscapeString(buf, model='%s', def-model); if (def-type == VIR_DOMAIN_SECLABEL_NONE) { virBufferAddLit(buf, /\n); @@ -17923,7 +17935,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, /devices\n); for (n = 0; n def-nseclabels; n++) -virSecurityLabelDefFormat(buf, def-seclabels[n], flags); +virSecurityLabelDefFormat(buf, def-seclabels[n]); if (def-namespaceData def-ns.format) { if ((def-ns.format)(buf, def-namespaceData) 0) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml new file mode 100644 index 000..cec59f8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.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' cpuset='1-4,8-20,525'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='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='pci' index='0' model='pci-root'/ +memballoon model='virtio'/ + /devices + seclabel type='none' model='none'/ +/domain diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 26e3cad..9f919de 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -307,6 +307,7 @@ mymain(void) DO_TEST_FULL(seclabel-static-labelskip, false, WHEN_ACTIVE);
[libvirt] [PATCH 3/3] virSecurityLabelDef: use enum type for @type
There's this trend in libvirt of using enum types wherever possible. Now that I'm at virSecurityLabelDef let's rework @type item of the structure so we don't have to typecast it elsewhere. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 6 -- src/security/security_dac.c | 2 +- src/util/virseclabel.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index de60cd2..b3a0ec8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4567,12 +4567,14 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, p = virXPathStringLimit(string(./@type), VIR_SECURITY_LABEL_BUFLEN - 1, ctxt); if (p) { -seclabel-type = virDomainSeclabelTypeFromString(p); -if (seclabel-type = 0) { +int type; /* virDomainSeclabelType */ +type = virDomainSeclabelTypeFromString(p); +if (type = 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(invalid security type '%s'), p); goto error; } +seclabel-type = type; } if (seclabel-type == VIR_DOMAIN_SECLABEL_STATIC || diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 4d2a9d6..b4bfc57 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1125,7 +1125,7 @@ virSecurityDACGenLabel(virSecurityManagerPtr mgr, return rc; } -switch ((virDomainSeclabelType) seclabel-type) { +switch (seclabel-type) { case VIR_DOMAIN_SECLABEL_STATIC: if (seclabel-label == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h index 94c4dfc..abf9510 100644 --- a/src/util/virseclabel.h +++ b/src/util/virseclabel.h @@ -39,7 +39,7 @@ struct _virSecurityLabelDef { char *label;/* security label string */ char *imagelabel; /* security image label string */ char *baselabel;/* base name of label string */ -int type; /* virDomainSeclabelType */ +virDomainSeclabelType type; /* seclabel @type */ bool relabel; /* true (default) for allowing relabels */ bool implicit; /* true if seclabel is auto-added */ }; -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 0/3] Couple of seclabels improvements
Some patches of the previous version are pushed already. However, there are some new too. Michal Privoznik (3): conf: Always format seclabel's model virSecurityLabelDefParseXML: Rework virSecurityLabelDef: use enum type for @type src/conf/domain_conf.c | 129 +++-- src/security/security_dac.c| 2 +- src/util/virseclabel.h | 2 +- .../qemuxml2argv-seclabel-dynamic-none.xml | 28 + tests/qemuxml2xmltest.c| 1 + 5 files changed, 101 insertions(+), 61 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] virSecurityLabelDefParseXML: Rework
Instead of allocating the virSecurityLabelDef structure ourselves, we can utilize virSecurityLabelDefNew which even sets the default values for us. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 103 - 1 file changed, 50 insertions(+), 53 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7b90903..de60cd2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4553,91 +4553,87 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, unsigned int flags) { char *p; -virSecurityLabelDefPtr def = NULL; +virSecurityLabelDefPtr seclabel = NULL; -if (VIR_ALLOC(def) 0) +p = virXPathStringLimit(string(./@model), +VIR_SECURITY_MODEL_BUFLEN - 1, ctxt); + +if (!(seclabel = virSecurityLabelDefNew(p))) goto error; +/* set seclabelault value */ +seclabel-type = VIR_DOMAIN_SECLABEL_DYNAMIC; + p = virXPathStringLimit(string(./@type), -VIR_SECURITY_LABEL_BUFLEN-1, ctxt); -if (p == NULL) { -def-type = VIR_DOMAIN_SECLABEL_DYNAMIC; -} else { -def-type = virDomainSeclabelTypeFromString(p); -VIR_FREE(p); -if (def-type = 0) { +VIR_SECURITY_LABEL_BUFLEN - 1, ctxt); +if (p) { +seclabel-type = virDomainSeclabelTypeFromString(p); +if (seclabel-type = 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - %s, _(invalid security type)); + _(invalid security type '%s'), p); goto error; } } +if (seclabel-type == VIR_DOMAIN_SECLABEL_STATIC || +seclabel-type == VIR_DOMAIN_SECLABEL_NONE) +seclabel-relabel = false; + +VIR_FREE(p); p = virXPathStringLimit(string(./@relabel), VIR_SECURITY_LABEL_BUFLEN-1, ctxt); -if (p != NULL) { +if (p) { if (STREQ(p, yes)) { -def-relabel = true; +seclabel-relabel = true; } else if (STREQ(p, no)) { -def-relabel = false; +seclabel-relabel = false; } else { virReportError(VIR_ERR_XML_ERROR, _(invalid security relabel value %s), p); -VIR_FREE(p); goto error; } -VIR_FREE(p); -if (def-type == VIR_DOMAIN_SECLABEL_DYNAMIC -!def-relabel) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - %s, _(dynamic label type must use resource relabeling)); -goto error; -} -if (def-type == VIR_DOMAIN_SECLABEL_NONE -def-relabel) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - %s, _(resource relabeling is not compatible with 'none' label type)); -goto error; -} -} else { -if (def-type == VIR_DOMAIN_SECLABEL_STATIC || -def-type == VIR_DOMAIN_SECLABEL_NONE) -def-relabel = false; -else -def-relabel = true; } -/* Always parse model */ -p = virXPathStringLimit(string(./@model), -VIR_SECURITY_MODEL_BUFLEN-1, ctxt); -def-model = p; +if (seclabel-type == VIR_DOMAIN_SECLABEL_DYNAMIC +!seclabel-relabel) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + %s, _(dynamic label type must use resource relabeling)); +goto error; +} +if (seclabel-type == VIR_DOMAIN_SECLABEL_NONE +seclabel-relabel) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + %s, _(resource relabeling is not compatible with 'none' label type)); +goto error; +} /* For the model 'none' none of the following labels is going to be * present. Hence, return now. */ -if (STREQ_NULLABLE(def-model, none)) { +if (STREQ_NULLABLE(seclabel-model, none)) { if (flags VIR_DOMAIN_XML_INACTIVE) { /* Fix older configurations */ -def-type = VIR_DOMAIN_SECLABEL_NONE; -def-relabel = false; +seclabel-type = VIR_DOMAIN_SECLABEL_NONE; +seclabel-relabel = false; } else { -if (def-type != VIR_DOMAIN_SECLABEL_NONE) { +if (seclabel-type != VIR_DOMAIN_SECLABEL_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(unsupported type='%s' to model 'none'), - virDomainSeclabelTypeToString(def-type)); + virDomainSeclabelTypeToString(seclabel-type)); goto error; } /* combination of relabel='yes' and type='static' * is checked a few lines above. */ } -return def; +return seclabel; }
[libvirt] [PATCH] schema: pool: netfs: Don't enforce slash in glusterfs pool source
Gluster volumes don't start with a leading slash. Our schema for netfs gluster pools enforces it though. Luckily mount.glusterfs skips it. Allow a slashless volume name for glusterfs netfs mounts in the schema. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1101999 --- docs/schemas/basictypes.rng| 6 +++ docs/schemas/storagepool.rng | 44 +- .../pool-netfs-gluster-without-slash.xml | 12 ++ 3 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-gluster-without-slash.xml diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 5fe3a97..9c9419f 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -231,6 +231,12 @@ /data /define + define name=dirPath +data type=string + param name=pattern[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param +/data + /define + define name=absFilePath data type=string param name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%,]+/param diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 8d7a94d..b2d1473 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -327,6 +327,15 @@ /element /define + define name='sourceinfonetfsgluster' +element name='dir' + attribute name='path' +ref name='dirPath'/ + /attribute + empty/ +/element + /define + define name='sourceinfoname' element name='name' text/ @@ -394,7 +403,6 @@ valueauto/value valuenfs/value valuecifs/value -valueglusterfs/value /choice /attribute /element @@ -468,14 +476,32 @@ define name='sourcenetfs' element name='source' - interleave -ref name='sourceinfohost'/ -ref name='sourceinfodir'/ -ref name='sourcefmtnetfs'/ -optional - ref name='sourceinfovendor'/ -/optional - /interleave + choice +group + interleave +ref name='sourceinfohost'/ +ref name='sourceinfodir'/ +ref name='sourcefmtnetfs'/ +optional +ref name='sourceinfovendor'/ +/optional + /interleave +/group +group + interleave +ref name='sourceinfohost'/ +ref name='sourceinfonetfsgluster'/ +element name='format' + attribute name='type' +valueglusterfs/value + /attribute +/element +optional +ref name='sourceinfovendor'/ +/optional + /interleave +/group + /choice /element /define diff --git a/tests/storagepoolxml2xmlin/pool-netfs-gluster-without-slash.xml b/tests/storagepoolxml2xmlin/pool-netfs-gluster-without-slash.xml new file mode 100644 index 000..69a2c6d --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-netfs-gluster-without-slash.xml @@ -0,0 +1,12 @@ +pool type='netfs' + source +host name='example.com'/ +format type='glusterfs'/ +dir path='volume'/ + /source + namenetfs-gluster/name + uuidd5609ced-94b1-489e-b218-eff35c30336a/uuid + target +path/mnt/gluster/path + /target +/pool -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Couple of seclabels improvements
On 07/10/2014 10:04 AM, Michal Privoznik wrote: diff to v1: - rework the 3rd patch - introduce one more bugfix Michal Privoznik (4): virSecurityLabelDef: substitute 'norelabel' with 'relabel' virSecurityDeviceLabelDef: substitute 'norelabel' with 'relabel' conf: Always format seclabel's model conf: Don't allow multiple seclabels for same model src/conf/domain_conf.c | 67 -- src/security/security_apparmor.c | 10 ++-- src/security/security_dac.c| 22 +++ src/security/security_manager.c| 2 +- src/security/security_selinux.c| 32 +-- src/util/virseclabel.c | 2 +- src/util/virseclabel.h | 4 +- .../qemuxml2argv-seclabel-dynamic-none.xml | 28 + .../qemuxml2argv-seclabel-multiple.xml | 40 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c| 1 + 11 files changed, 142 insertions(+), 67 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-multiple.xml There's a Coverity issue from these patches - it looks like perhaps patch 12 were combined when submitted into commit id '13adf1b' which has: virSecurityLabelDefPtr virSecurityLabelDefNew(const char *model) { virSecurityLabelDefPtr seclabel = NULL; if (VIR_ALLOC(seclabel) 0 || VIR_STRDUP(seclabel-model, model) 0) { virSecurityLabelDefFree(seclabel); seclabel = NULL; } +seclabel-relabel = true; + return seclabel; } See the problem at all? It's a FORWARD_NULL on 'seclabel'. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] vhost-user support: domain configuration
On 10/07/2014 18:01, Michal Privoznik wrote: On 02.07.2014 15:20, Michele Paolino wrote: vhost-user is added as a virDomainChrSourceDefPtr variable in the virtual network interface configuration structure. This patch adds support to parse vhost-user element. The XML file looks like: interface type='vhostuser' source type='unix' path='/tmp/vhost.sock' mode='server'/ mac address='52:54:00:3b:83:1a'/ model type='virtio'/ /interface Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- src/conf/domain_conf.c | 81 ++ src/conf/domain_conf.h | 10 +-- 2 files changed, 89 insertions(+), 2 deletions(-) Introducing a new device (subtype of a device) must always go hand in hand with documentation and XML schema adjustment. Moreover, it would be nice to test the new XML (e.g. domainschematest or qemuxml2argv stub (a .xml file under tests/qemuxml2argvdata without .args counterpart which will be introduced once qemu implementation is done)). Yes, these files are present in 3/4. I will post the v2 of this series in a single patch. diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ea09bdc..de52ca4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy, VIR_DOMAIN_FS_WRPOLICY_LAST, VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, user, ethernet, + vhostuser, server, client, mcast, @@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def-data.ethernet.ipaddr); break; +case VIR_DOMAIN_NET_TYPE_VHOSTUSER: +virDomainChrSourceDefFree(def-data.vhostuser); +break; + case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: @@ -6665,6 +6670,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *mode = NULL; char *linkstate = NULL; char *addrtype = NULL; +char *vhostuser_mode = NULL; +char *vhostuser_path = NULL; +char *vhostuser_type = NULL; virNWFilterHashTablePtr filterparams = NULL; virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt-node; @@ -6710,6 +6718,21 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur-name, BAD_CAST source)) { dev = virXMLPropString(cur, dev); mode = virXMLPropString(cur, mode); +} else if (!vhostuser_path !vhostuser_mode !vhostuser_type +def-type == VIR_DOMAIN_NET_TYPE_VHOSTUSER + xmlStrEqual(cur-name, BAD_CAST source)) { +vhostuser_type = virXMLPropString(cur, type); +if (!vhostuser_type || STREQ(vhostuser_type, unix)) { +vhostuser_path = virXMLPropString(cur, path); +vhostuser_mode = virXMLPropString(cur, mode); +} +else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(type='%s' unsupported for +interface type='vhostuser'), + vhostuser_type); +goto error; I'd move this check a few lines below to the other checks. The check has been done here because if in future we will decide to support other chardevs in addition to the unix socket, the XML file will be different. For example, if we want to add the support for a TCP socket, the path attribute needs to be replaced with host, service and protocol. After a quick look at the _virDomainChrSourceDef structure, the XML description in this case should look like: source type='tcp' host='host' service='serv' mode='mode' protocol='prot'/ Do we want to add these additional checks only when the support for other chardevs will be added, or is there an alternative solution? +} } else if (!def-virtPortProfile xmlStrEqual(cur-name, BAD_CAST virtualport)) { if (def-type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -6867,6 +6890,50 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, actual = NULL; break; +case VIR_DOMAIN_NET_TYPE_VHOSTUSER: +if (!model || STRNEQ(model, virtio)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Wrong or no model 'type' attribute + specified with interface type='vhostuser'/. + vhostuser requires the virtio-net* frontend)); +goto error; +} + +if (VIR_ALLOC(def-data.vhostuser) 0) +goto error; + +if (STREQ(vhostuser_type, unix)) { Ouch, in the code I've commented above,
Re: [libvirt] [PATCH 3/4] vhost-user support: tests and docs
On 10/07/2014 18:01, Michal Privoznik wrote: On 02.07.2014 15:20, Michele Paolino wrote: This patch adds the test files and the documentation for vhost-user. Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- docs/formatdomain.html.in | 34 +++ docs/schemas/domaincommon.rng | 39 ++ .../qemuxml2argv-net-vhostuser.args| 7 .../qemuxml2argv-net-vhostuser.xml | 33 ++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c| 1 + 6 files changed, 115 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml Aha! You are doing here what I've just requested in 1/4. Still I rather see it in a single patch (even though it will be bigger) because it make maintainers life easier. You know, backporting a single patch versus digging through 'git log' to search for this commit ... diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3f8bbee..606b7d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3927,6 +3927,40 @@ qemu-kvm -net nic,model=? /dev/null span class=sinceSince 0.9.5/span /p +h5a name=elementVhostuservhost-user interface/a/h5 + +p + vhost-user enables the communication between a QEMU virtual machine + and other userspace process using the Virtio transport protocol. + A char dev (e.g. Unix socket) is used for the control plane, while + the data plane is based on shared memory. +/p + +pre + ... + lt;devicesgt; +lt;interface type='vhostuser'gt; + lt;source type='unix' path='/tmp/vhost.sock' mode='server'gt; + lt;/sourcegt; + lt;mac address='52:54:00:3b:83:1a'gt; + lt;/macgt; + lt;model type='virtio'gt; + lt;/modelgt; +lt;/interfacegt; + lt;/devicesgt; + .../pre + +p + The codelt;sourcegt;/code element has to be specified + along with the type of char device. + Currently, only type='unix' is supported, where the path (the + directory path of the socket) and mode attributes are required. + Both codemode='server'/code and codemode='client'/code + are supported. + vhost-user requires the virtio model type, thus the + codelt;modelgt;/code element is mandatory. +/p + h4a name=elementsInputInput devices/a/h4 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af51eee..f85dd61 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1968,6 +1968,45 @@ /group group attribute name=type +valuevhostuser/value + /attribute + interleave +optional + element name=source I wouldn't say source/ is optional in case of interface type='vhostnet'/. It contains crucial information that helps us construct the correct qemu command line which would not be possible otherwise. I agree, but the schemas of the other interfaces it's the same. Please see the Laine's comment at http://www.redhat.com/archives/libvir-list/2014-June/msg00279.html +attribute name=type + choice +valuenull/value +valuevc/value +valuepty/value +valuedev/value +valuefile/value +valuepipe/value +valuestdio/value +valueudp/value +valuetcp/value +valueunix/value +valuespicevmc/value +valuespiceport/value +valuenmdm/value Honestly, I'm inclined to not enumerate all of these here now. I mean, only 'unix' is supported now. On the other hand, it's acceptable to have a RNG schema that allows something more than our XML parser. + /choice +/attribute +attribute name=path + ref name=absFilePath/ +/attribute +attribute name=mode + choice +valueserver/value +valueclient/value + /choice +/attribute +empty/ + /element +/optional +ref name=interface-options/ + /interleave +/group +group + attribute name=type valuenetwork/value /attribute interleave diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args new file mode 100644 index 000..cc66ec3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args @@
Re: [libvirt] [PATCH 3/4] vhost-user support: tests and docs
On 11.07.2014 13:07, Michele Paolino wrote: On 10/07/2014 18:01, Michal Privoznik wrote: On 02.07.2014 15:20, Michele Paolino wrote: This patch adds the test files and the documentation for vhost-user. Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- docs/formatdomain.html.in | 34 +++ docs/schemas/domaincommon.rng | 39 ++ .../qemuxml2argv-net-vhostuser.args| 7 .../qemuxml2argv-net-vhostuser.xml | 33 ++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c| 1 + 6 files changed, 115 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af51eee..f85dd61 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1968,6 +1968,45 @@ /group group attribute name=type +valuevhostuser/value + /attribute + interleave +optional + element name=source I wouldn't say source/ is optional in case of interface type='vhostnet'/. It contains crucial information that helps us construct the correct qemu command line which would not be possible otherwise. I agree, but the schemas of the other interfaces it's the same. Please see the Laine's comment at http://www.redhat.com/archives/libvir-list/2014-June/msg00279.html Well, not for all other interface types. Moreover, currently the your patches require source/. But I can live with schema wider than our parser. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] vhost-user support: domain configuration
On 11.07.2014 13:06, Michele Paolino wrote: On 10/07/2014 18:01, Michal Privoznik wrote: On 02.07.2014 15:20, Michele Paolino wrote: vhost-user is added as a virDomainChrSourceDefPtr variable in the virtual network interface configuration structure. This patch adds support to parse vhost-user element. The XML file looks like: interface type='vhostuser' source type='unix' path='/tmp/vhost.sock' mode='server'/ mac address='52:54:00:3b:83:1a'/ model type='virtio'/ /interface Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- src/conf/domain_conf.c | 81 ++ src/conf/domain_conf.h | 10 +-- 2 files changed, 89 insertions(+), 2 deletions(-) Introducing a new device (subtype of a device) must always go hand in hand with documentation and XML schema adjustment. Moreover, it would be nice to test the new XML (e.g. domainschematest or qemuxml2argv stub (a .xml file under tests/qemuxml2argvdata without .args counterpart which will be introduced once qemu implementation is done)). Yes, these files are present in 3/4. I will post the v2 of this series in a single patch. diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ea09bdc..de52ca4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy, VIR_DOMAIN_FS_WRPOLICY_LAST, VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, user, ethernet, + vhostuser, server, client, mcast, @@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def-data.ethernet.ipaddr); break; +case VIR_DOMAIN_NET_TYPE_VHOSTUSER: +virDomainChrSourceDefFree(def-data.vhostuser); +break; + case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: @@ -6665,6 +6670,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *mode = NULL; char *linkstate = NULL; char *addrtype = NULL; +char *vhostuser_mode = NULL; +char *vhostuser_path = NULL; +char *vhostuser_type = NULL; virNWFilterHashTablePtr filterparams = NULL; virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt-node; @@ -6710,6 +6718,21 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur-name, BAD_CAST source)) { dev = virXMLPropString(cur, dev); mode = virXMLPropString(cur, mode); +} else if (!vhostuser_path !vhostuser_mode !vhostuser_type +def-type == VIR_DOMAIN_NET_TYPE_VHOSTUSER + xmlStrEqual(cur-name, BAD_CAST source)) { +vhostuser_type = virXMLPropString(cur, type); +if (!vhostuser_type || STREQ(vhostuser_type, unix)) { +vhostuser_path = virXMLPropString(cur, path); +vhostuser_mode = virXMLPropString(cur, mode); +} +else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(type='%s' unsupported for +interface type='vhostuser'), + vhostuser_type); +goto error; I'd move this check a few lines below to the other checks. The check has been done here because if in future we will decide to support other chardevs in addition to the unix socket, the XML file will be different. Until then I find the code easier to read with checks grouped. For example, if we want to add the support for a TCP socket, the path attribute needs to be replaced with host, service and protocol. After a quick look at the _virDomainChrSourceDef structure, the XML description in this case should look like: source type='tcp' host='host' service='serv' mode='mode' protocol='prot'/ Do we want to add these additional checks only when the support for other chardevs will be added, or is there an alternative solution? Yes. There's no need to introduce the checks for unsupported combinations now. +} } else if (!def-virtPortProfile xmlStrEqual(cur-name, BAD_CAST virtualport)) { if (def-type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -6867,6 +6890,50 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, actual = NULL; break; +case VIR_DOMAIN_NET_TYPE_VHOSTUSER: +if (!model || STRNEQ(model, virtio)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Wrong or no model 'type' attribute + specified with interface type='vhostuser'/. + vhostuser requires the virtio-net* frontend)); +goto error; +
Re: [libvirt] [PATCH] leaseshelper: add enhancements to support all events
On Fri, Jul 11, 2014 at 5:49 AM, Nehal J Wani nehaljw.k...@gmail.com wrote: This patch enables the helper program to detect event(s) triggered when there is a change in lease length or expiry and client-id. This transfers complete control of leases database to libvirt and suppresses use of the lease database file (network-name.leases). That file will not be created, read, or written. This is achieved by adding the option --leasefile-ro to dnsmasq and applying the symlink technique, which helps us map events related to leases with their corresponding network bridges. Example: /var/lib/libvirt/dnsmasq/virbr0.helper - /home/wani/libvirt/src/libvirt_leaseshelper /var/lib/libvirt/dnsmasq/virbr3.helper - /home/wani/libvirt/src/libvirt_leaseshelper Also, this requires the addition of a new non-lease entry in our custom lease database: server-duid. It is required to identify a DHCPv6 server. Now that dnsmasq doesn't maintain its own leases database, it relies on our helper program to tell it about previous leases and server duid. Thus, this patch makes our leases program honor an extra action: init, in which it sends the known info in a particular format to dnsmasq by printing it to stdout. --- src/network/bridge_driver.c | 43 +++- src/network/leaseshelper.c | 156 +--- 2 files changed, 175 insertions(+), 24 deletions(-) make syntax-check reported two flaws. Following diff rectifies it: diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index 3d6c773..bdc1b77 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -30,6 +30,7 @@ #include stdlib.h #include sys/stat.h +#include dirname.h #include virutil.h #include virthread.h #include virfile.h @@ -54,7 +55,7 @@ * Use this when passing possibly-NULL strings to printf-a-likes. * Required for unkown parameters during init call. */ -# define EMPTY_STR(s) ((s) ? (s) : *) +#define EMPTY_STR(s) ((s) ? (s) : *) static const char *program_name; @@ -181,7 +182,7 @@ main(int argc, char **argv) * expired leases. Our symlink hack provides us an alternative method to * find it */ if (!interface) { -interface = basename(argv[0]); +interface = last_component(argv[0]); *(strchr(interface, '.')) = '\0'; } Regards, Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] LXC: add support for --config in setmem command
On 07/11/2014 11:26 AM, Chen Hanxiao wrote: In lxc, we could not use setmem command with --config options. This patch will add support for this. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v3: add max_balloon check for AFFECT_CONFIG v2: use virDomainSetMemoryFlagsEnsureACL remove redundant domain running check src/lxc/lxc_driver.c | 58 ++-- 1 file changed, 47 insertions(+), 11 deletions(-) ACK and pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] storage: Don't wipe volumes on remote filesystems with local tools
Version 2 now splits the stuff into separate driver backend funcs. Peter Krempa (2): storage: wipe: Move helper code into storage backend storage: Split out volume wiping as separate backend function src/storage/storage_backend.c | 203 + src/storage/storage_backend.h | 12 ++ src/storage/storage_backend_disk.c| 1 + src/storage/storage_backend_fs.c | 3 + src/storage/storage_backend_iscsi.c | 1 + src/storage/storage_backend_logical.c | 1 + src/storage/storage_backend_mpath.c | 1 + src/storage/storage_backend_scsi.c| 1 + src/storage/storage_driver.c | 205 +- 9 files changed, 229 insertions(+), 199 deletions(-) -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] storage: Split out volume wiping as separate backend function
For non-local storage drivers we can't expect to use the scrub tool to wipe the volume. Split the code into a separate backend function so that we can add protocol specific code later. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1118710 --- src/storage/storage_backend.h | 6 ++ src/storage/storage_backend_disk.c| 1 + src/storage/storage_backend_fs.c | 3 +++ src/storage/storage_backend_iscsi.c | 1 + src/storage/storage_backend_logical.c | 1 + src/storage/storage_backend_mpath.c | 1 + src/storage/storage_backend_scsi.c| 1 + src/storage/storage_driver.c | 10 +++--- 8 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 5e251d7..e48da5b 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -87,6 +87,11 @@ typedef int (*virStorageBackendVolumeUpload)(virConnectPtr conn, unsigned long long offset, unsigned long long len, unsigned int flags); +typedef int (*virStorageBackendVolumeWipe)(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned int algorithm, + unsigned int flags); /* File creation/cloning functions used for cloning between backends */ int virStorageBackendCreateRaw(virConnectPtr conn, @@ -150,6 +155,7 @@ struct _virStorageBackend { virStorageBackendVolumeResize resizeVol; virStorageBackendVolumeUpload uploadVol; virStorageBackendVolumeDownload downloadVol; +virStorageBackendVolumeWipe wipeVol; }; virStorageBackendPtr virStorageBackendForType(int type); diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index f900dee..d85f13f 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -794,4 +794,5 @@ virStorageBackend virStorageBackendDisk = { .buildVolFrom = virStorageBackendDiskBuildVolFrom, .uploadVol = virStorageBackendVolUploadLocal, .downloadVol = virStorageBackendVolDownloadLocal, +.wipeVol = virStorageBackendVolWipeLocal, }; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 1615c12..58e849f 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1291,6 +1291,7 @@ virStorageBackend virStorageBackendDirectory = { .resizeVol = virStorageBackendFileSystemVolResize, .uploadVol = virStorageBackendVolUploadLocal, .downloadVol = virStorageBackendVolDownloadLocal, +.wipeVol = virStorageBackendVolWipeLocal, }; #if WITH_STORAGE_FS @@ -1311,6 +1312,7 @@ virStorageBackend virStorageBackendFileSystem = { .resizeVol = virStorageBackendFileSystemVolResize, .uploadVol = virStorageBackendVolUploadLocal, .downloadVol = virStorageBackendVolDownloadLocal, +.wipeVol = virStorageBackendVolWipeLocal, }; virStorageBackend virStorageBackendNetFileSystem = { .type = VIR_STORAGE_POOL_NETFS, @@ -1330,6 +1332,7 @@ virStorageBackend virStorageBackendNetFileSystem = { .resizeVol = virStorageBackendFileSystemVolResize, .uploadVol = virStorageBackendVolUploadLocal, .downloadVol = virStorageBackendVolDownloadLocal, +.wipeVol = virStorageBackendVolWipeLocal, }; diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 7345571..1d0cf73 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -476,4 +476,5 @@ virStorageBackend virStorageBackendISCSI = { .findPoolSources = virStorageBackendISCSIFindPoolSources, .uploadVol = virStorageBackendVolUploadLocal, .downloadVol = virStorageBackendVolDownloadLocal, +.wipeVol = virStorageBackendVolWipeLocal, }; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index faa9a4b..562f019 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -843,4 +843,5 @@ virStorageBackend virStorageBackendLogical = { .deleteVol = virStorageBackendLogicalDeleteVol, .uploadVol = virStorageBackendVolUploadLocal, .downloadVol = virStorageBackendVolDownloadLocal, +.wipeVol = virStorageBackendVolWipeLocal, }; diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 402faa0..f21ae4c 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -289,4 +289,5 @@ virStorageBackend virStorageBackendMpath = { .refreshPool = virStorageBackendMpathRefreshPool, .uploadVol = virStorageBackendVolUploadLocal, .downloadVol = virStorageBackendVolDownloadLocal, +.wipeVol =
[libvirt] [PATCHv2 1/2] storage: wipe: Move helper code into storage backend
The next patch will move the storage volume wiping code into the individual backends. This patch splits out the common code to wipe a local volume into a separate backend helper so that the next patch is simpler. --- src/storage/storage_backend.c | 203 ++ src/storage/storage_backend.h | 6 ++ src/storage/storage_driver.c | 199 + 3 files changed, 210 insertions(+), 198 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 7b17ca4..8e62403 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1700,6 +1700,209 @@ virStorageBackendVolDownloadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, return virFDStreamOpenFile(stream, vol-target.path, offset, len, O_RDONLY); } + +/* If the volume we're wiping is already a sparse file, we simply + * truncate and extend it to its original size, filling it with + * zeroes. This behavior is guaranteed by POSIX: + * + * http://www.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html + * + * If fildes refers to a regular file, the ftruncate() function shall + * cause the size of the file to be truncated to length. If the size + * of the file previously exceeded length, the extra data shall no + * longer be available to reads on the file. If the file previously + * was smaller than this size, ftruncate() shall increase the size of + * the file. If the file size is increased, the extended area shall + * appear as if it were zero-filled. + */ +static int +virStorageBackendVolZeroSparseFileLocal(virStorageVolDefPtr vol, +off_t size, +int fd) +{ +int ret = -1; + +ret = ftruncate(fd, 0); +if (ret == -1) { +virReportSystemError(errno, + _(Failed to truncate volume with + path '%s' to 0 bytes), + vol-target.path); +return ret; +} + +ret = ftruncate(fd, size); +if (ret == -1) { +virReportSystemError(errno, + _(Failed to truncate volume with + path '%s' to %ju bytes), + vol-target.path, (uintmax_t)size); +} + +return ret; +} + + +static int +virStorageBackendWipeExtentLocal(virStorageVolDefPtr vol, + int fd, + off_t extent_start, + off_t extent_length, + char *writebuf, + size_t writebuf_length, + size_t *bytes_wiped) +{ +int ret = -1, written = 0; +off_t remaining = 0; +size_t write_size = 0; + +VIR_DEBUG(extent logical start: %ju len: %ju, + (uintmax_t)extent_start, (uintmax_t)extent_length); + +if ((ret = lseek(fd, extent_start, SEEK_SET)) 0) { +virReportSystemError(errno, + _(Failed to seek to position %ju in volume + with path '%s'), + (uintmax_t)extent_start, vol-target.path); +goto cleanup; +} + +remaining = extent_length; +while (remaining 0) { + +write_size = (writebuf_length remaining) ? writebuf_length : remaining; +written = safewrite(fd, writebuf, write_size); +if (written 0) { +virReportSystemError(errno, + _(Failed to write %zu bytes to + storage volume with path '%s'), + write_size, vol-target.path); + +goto cleanup; +} + +*bytes_wiped += written; +remaining -= written; +} + +if (fdatasync(fd) 0) { +ret = -errno; +virReportSystemError(errno, + _(cannot sync data to volume with path '%s'), + vol-target.path); +goto cleanup; +} + +VIR_DEBUG(Wrote %zu bytes to volume with path '%s', + *bytes_wiped, vol-target.path); + +ret = 0; + + cleanup: +return ret; +} + + +int +virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol, + unsigned int algorithm, + unsigned int flags) +{ +int ret = -1, fd = -1; +struct stat st; +char *writebuf = NULL; +size_t bytes_wiped = 0; +virCommandPtr cmd = NULL; + +virCheckFlags(0, -1); + +VIR_DEBUG(Wiping volume with path '%s' and algorithm %u, + vol-target.path, algorithm); + +fd = open(vol-target.path, O_RDWR); +if (fd == -1) { +virReportSystemError(errno, + _(Failed to open
Re: [libvirt] CPU model API (v2)
On Thu, Jul 10, 2014 at 9:47 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: v2: * Correct hierarchy for GVirConfigDomainCpuModel * Correct order of new symbols in .sym file Ouch, I messed-up the git-send-email commandline and ended-up without the 'libvirt-glib' prefix to subject lines. Sorry for that. -- Regards, Zeeshan Ali (Khattak) Befriend GNOME: http://www.gnome.org/friends/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/8] hostdev: Introduce virDomainHostdevSubsysSCSI
Create a separate typedef for the hostdev union data describing SCSI Then adjust the code to use the new pointer Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_audit.c | 7 +++--- src/conf/domain_conf.c | 40 -- src/conf/domain_conf.h | 18 -- src/qemu/qemu_cgroup.c | 7 +++--- src/qemu/qemu_command.c | 7 +++--- src/qemu/qemu_conf.c | 18 -- src/qemu/qemu_hotplug.c | 12 - src/security/security_apparmor.c | 10 +++- src/security/security_dac.c | 20 ++- src/security/security_selinux.c | 20 ++- src/util/virhostdev.c| 53 11 files changed, 98 insertions(+), 114 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index d3f0449..370dc3a 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -390,6 +390,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, const char *virt; virDomainHostdevSubsysUSBPtr usbsrc = hostdev-source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = hostdev-source.subsys.u.pci; +virDomainHostdevSubsysSCSIPtr scsisrc = hostdev-source.subsys.u.scsi; virUUIDFormat(vm-def-uuid, uuidstr); if (!(vmname = virAuditEncode(vm, vm-def-name))) { @@ -424,10 +425,8 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: if (virAsprintfQuiet(address, %s:%d:%d:%d, - hostdev-source.subsys.u.scsi.adapter, - hostdev-source.subsys.u.scsi.bus, - hostdev-source.subsys.u.scsi.target, - hostdev-source.subsys.u.scsi.unit) 0) { + scsisrc-adapter, scsisrc-bus, + scsisrc-target, scsisrc-unit) 0) { VIR_WARN(OOM while encoding audit message); goto cleanup; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f1b9a46..e38771c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4025,6 +4025,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, bool got_address = false, got_adapter = false; xmlNodePtr cur; char *bus = NULL, *target = NULL, *unit = NULL; +virDomainHostdevSubsysSCSIPtr scsisrc = def-source.subsys.u.scsi; cur = node-children; while (cur != NULL) { @@ -4046,19 +4047,19 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, goto cleanup; } -if (virStrToLong_ui(bus, NULL, 0, def-source.subsys.u.scsi.bus) 0) { +if (virStrToLong_ui(bus, NULL, 0, scsisrc-bus) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse bus '%s'), bus); goto cleanup; } -if (virStrToLong_ui(target, NULL, 0, def-source.subsys.u.scsi.target) 0) { +if (virStrToLong_ui(target, NULL, 0, scsisrc-target) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse target '%s'), target); goto cleanup; } -if (virStrToLong_ui(unit, NULL, 0, def-source.subsys.u.scsi.unit) 0) { +if (virStrToLong_ui(unit, NULL, 0, scsisrc-unit) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse unit '%s'), unit); goto cleanup; @@ -4072,8 +4073,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, for scsi hostdev source)); goto cleanup; } -if (!(def-source.subsys.u.scsi.adapter = - virXMLPropString(cur, name))) { +if (!(scsisrc-adapter = virXMLPropString(cur, name))) { virReportError(VIR_ERR_XML_ERROR, %s, _('adapter' must be specified for scsi hostdev source)); goto cleanup; @@ -4254,6 +4254,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, int backend; int ret = -1; virDomainHostdevSubsysPCIPtr pcisrc = def-source.subsys.u.pci; +virDomainHostdevSubsysSCSIPtr scsisrc = def-source.subsys.u.scsi; /* @managed can be read from the xml document - it is always an * attribute of the toplevel element, no matter what type of @@ -4311,8 +4312,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, goto error; } -if ((def-source.subsys.u.scsi.sgio = - virDomainDeviceSGIOTypeFromString(sgio)) = 0) { +if
[libvirt] [PATCH 4/8] hostdev: Introduce virDomainHostdevSubsysSCSIHost
Split virDomainHostdevSubsysSCSI further. In preparation for having either SCSI or iSCSI data, create a union in virDomainHostdevSubsysSCSI to contain just a virDomainHostdevSubsysSCSIHost to describe the 'scsi_host' host device Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_audit.c | 8 +++--- src/conf/domain_conf.c | 58 src/conf/domain_conf.h | 14 +++--- src/qemu/qemu_cgroup.c | 9 --- src/qemu/qemu_command.c | 7 +++-- src/qemu/qemu_conf.c | 18 +++-- src/qemu/qemu_hotplug.c | 13 + src/security/security_apparmor.c | 5 ++-- src/security/security_dac.c | 10 --- src/security/security_selinux.c | 10 --- src/util/virhostdev.c| 36 - 11 files changed, 113 insertions(+), 75 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 370dc3a..ee9baa2 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -423,14 +423,16 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, goto cleanup; } break; -case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: +case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { +virDomainHostdevSubsysSCSIHostPtr scsihostsrc = scsisrc-u.host; if (virAsprintfQuiet(address, %s:%d:%d:%d, - scsisrc-adapter, scsisrc-bus, - scsisrc-target, scsisrc-unit) 0) { + scsihostsrc-adapter, scsihostsrc-bus, + scsihostsrc-target, scsihostsrc-unit) 0) { VIR_WARN(OOM while encoding audit message); goto cleanup; } break; +} default: VIR_WARN(Unexpected hostdev type while encoding audit message: %d, hostdev-source.subsys.type); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e38771c..55c0822 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1733,7 +1733,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) break; case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: if (def-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) -VIR_FREE(def-source.subsys.u.scsi.adapter); +VIR_FREE(def-source.subsys.u.scsi.u.host.adapter); break; } } @@ -4018,16 +4018,16 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, } static int -virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, - virDomainHostdevDefPtr def) +virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode, + virDomainHostdevSubsysSCSIPtr scsisrc) { int ret = -1; bool got_address = false, got_adapter = false; xmlNodePtr cur; char *bus = NULL, *target = NULL, *unit = NULL; -virDomainHostdevSubsysSCSIPtr scsisrc = def-source.subsys.u.scsi; +virDomainHostdevSubsysSCSIHostPtr scsihostsrc = scsisrc-u.host; -cur = node-children; +cur = sourcenode-children; while (cur != NULL) { if (cur-type == XML_ELEMENT_NODE) { if (xmlStrEqual(cur-name, BAD_CAST address)) { @@ -4047,19 +4047,20 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, goto cleanup; } -if (virStrToLong_ui(bus, NULL, 0, scsisrc-bus) 0) { +if (virStrToLong_ui(bus, NULL, 0, scsihostsrc-bus) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse bus '%s'), bus); goto cleanup; } -if (virStrToLong_ui(target, NULL, 0, scsisrc-target) 0) { +if (virStrToLong_ui(target, NULL, 0, +scsihostsrc-target) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse target '%s'), target); goto cleanup; } -if (virStrToLong_ui(unit, NULL, 0, scsisrc-unit) 0) { +if (virStrToLong_ui(unit, NULL, 0, scsihostsrc-unit) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse unit '%s'), unit); goto cleanup; @@ -4073,7 +4074,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, for scsi hostdev source)); goto cleanup; } -if (!(scsisrc-adapter = virXMLPropString(cur, name))) { +if (!(scsihostsrc-adapter = virXMLPropString(cur, name))) { virReportError(VIR_ERR_XML_ERROR, %s,
[libvirt] [PATCH 0/8] Add iSCSI hostdev pass-through device
https://bugzilla.redhat.com/show_bug.cgi?id=957293 Allow iSCSI hostdev/ devices to be added similarly to the existing disk/ devices. Initially do a little bit of refactoring of the hostdev subsys structures to make it easier to model an iSCSI hostdev after the SCSI scsi_host. Although the bulk of the non-structure changes are technically unnecessary, it just looked nicer to see {usb|pci|scsi}src rather than the longer -source.subsys.u.{usb|pci|scsi} in many places in the code. The end result is the guest will have /dev/sdX devices created. I have run this code through my Coverity environment and will be running the virttest suite over the weekend. Patches 1-3 are repetitive moves of the various hostdev subsys types (USB, PCI, and SCSI) into separate typedefs and then modifying code use the Ptr instead of the long union path to each field. I think I got most, but I'm sure there's still a few that could be cleaned up or that were added since I started this. Patch 4 more or less redoes patch 3 and I suppose could be combined. I left it separate because it's showing the progression to patch 6. This patch uses 'scsihost' to reference the specific portions while still using 'scsisrc' to reference the shared portion (which is only sgio). Patch 5 adds a virConnectPtr since patch 6 will need a way to get the secret value for the iSCSI secret/auth in qemuBuildSCSIHostdevDrvStr. Patch 6 adds the qemu code to handle a new hostdev protocol type VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI. The code will mimic the network disk (VIR_STORAGE_NET_PROTOCOL_ISCSI) code when making decisions. The new 'scsisrc' field 'protocol' will be the decision point. The default of VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE was chosen over TYPE_SCSI_HOST. The changes were made in this order to reduce the size of the patch when the XML is added (patch 8). Most of the code was done inline using the iscsisrc-protocol (when the iSCSI code had nothing to do). The remaining changes refactored code into SCSIHost and SCSIiSCSI to process the differences. Patch 7 just refactors domain_conf to add what will be a common routine to handle the host/ element network disks. The iSCSI code will reuse the code to have it's own element. Patch 8 adds the XML, schema, and updates the docs to describe the hostdev/ entry as well as of course adding new tests. The tests are a copy/merge of the scsi_host and network iscsi tests to define and describe the expected model. John Ferlan (8): hostdev: Introduce virDomainHostdevSubsysUSB hostdev: Introduce virDomainHostdevSubsysPCI hostdev: Introduce virDomainHostdevSubsysSCSI hostdev: Introduce virDomainHostdevSubsysSCSIHost Add virConnectPtr for qemuBuildSCSIHostdevDrvStr hostdev: Introduce virDomainHostdevSubsysSCSIiSCSI domain_conf: Common routine to handle network storage host xml def hostdev: Add iSCSI hostdev XML docs/formatdomain.html.in | 142 +-- docs/schemas/domaincommon.rng | 46 +- src/conf/domain_audit.c| 38 +- src/conf/domain_conf.c | 463 +++-- src/conf/domain_conf.h | 80 +++- src/libxl/libxl_conf.c | 9 +- src/libxl/libxl_domain.c | 5 +- src/libxl/libxl_driver.c | 34 +- src/lxc/lxc_cgroup.c | 4 +- src/lxc/lxc_controller.c | 10 +- src/lxc/lxc_driver.c | 16 +- src/qemu/qemu_cgroup.c | 69 +-- src/qemu/qemu_command.c| 158 --- src/qemu/qemu_command.h| 5 +- src/qemu/qemu_conf.c | 20 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_hotplug.c| 93 +++-- src/qemu/qemu_hotplug.h| 9 +- src/security/security_apparmor.c | 33 +- src/security/security_dac.c| 66 +-- src/security/security_selinux.c| 64 +-- src/security/virt-aa-helper.c | 5 +- src/util/virhostdev.c | 295 +++-- .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args | 14 + .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml | 46 ++ .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args | 14 + .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml| 40 ++ ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args | 16 + ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml | 46 ++ .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args| 16 + .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml | 40 ++ tests/qemuxml2argvtest.c | 16 + tests/qemuxml2xmltest.c| 5 + 33 files changed, 1317 insertions(+), 606
[libvirt] [PATCH 2/8] hostdev: Introduce virDomainHostdevSubsysPCI
Create a separate typedef for the hostdev union data describing PCI. Then adjust the code to use the new pointer Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_audit.c | 9 src/conf/domain_conf.c | 25 - src/conf/domain_conf.h | 12 ++ src/libxl/libxl_conf.c | 9 src/libxl/libxl_domain.c | 5 +++-- src/libxl/libxl_driver.c | 34 ++--- src/qemu/qemu_cgroup.c | 24 ++-- src/qemu/qemu_command.c | 47 ++-- src/qemu/qemu_hotplug.c | 11 +- src/security/security_apparmor.c | 10 - src/security/security_dac.c | 20 +++-- src/security/security_selinux.c | 20 +++-- src/util/virhostdev.c| 34 - 13 files changed, 125 insertions(+), 135 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 8277b06..d3f0449 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -389,6 +389,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, char *device = NULL; const char *virt; virDomainHostdevSubsysUSBPtr usbsrc = hostdev-source.subsys.u.usb; +virDomainHostdevSubsysPCIPtr pcisrc = hostdev-source.subsys.u.pci; virUUIDFormat(vm-def-uuid, uuidstr); if (!(vmname = virAuditEncode(vm, vm-def-name))) { @@ -406,10 +407,10 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, switch (hostdev-source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: if (virAsprintfQuiet(address, %.4x:%.2x:%.2x.%.1x, - hostdev-source.subsys.u.pci.addr.domain, - hostdev-source.subsys.u.pci.addr.bus, - hostdev-source.subsys.u.pci.addr.slot, - hostdev-source.subsys.u.pci.addr.function) 0) { + pcisrc-addr.domain, + pcisrc-addr.bus, + pcisrc-addr.slot, + pcisrc-addr.function) 0) { VIR_WARN(OOM while encoding audit message); goto cleanup; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 046b4f8..f1b9a46 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4253,6 +4253,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, char *backendStr = NULL; int backend; int ret = -1; +virDomainHostdevSubsysPCIPtr pcisrc = def-source.subsys.u.pci; /* @managed can be read from the xml document - it is always an * attribute of the toplevel element, no matter what type of @@ -4332,7 +4333,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, has been specified), backendStr); goto error; } -def-source.subsys.u.pci.backend = backend; +pcisrc-backend = backend; break; @@ -10123,10 +10124,13 @@ static int virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr a, virDomainHostdevDefPtr b) { -if (a-source.subsys.u.pci.addr.domain == b-source.subsys.u.pci.addr.domain -a-source.subsys.u.pci.addr.bus == b-source.subsys.u.pci.addr.bus -a-source.subsys.u.pci.addr.slot == b-source.subsys.u.pci.addr.slot -a-source.subsys.u.pci.addr.function == b-source.subsys.u.pci.addr.function) +virDomainHostdevSubsysPCIPtr apcisrc = a-source.subsys.u.pci; +virDomainHostdevSubsysPCIPtr bpcisrc = b-source.subsys.u.pci; + +if (apcisrc-addr.domain == bpcisrc-addr.domain +apcisrc-addr.bus == bpcisrc-addr.bus +apcisrc-addr.slot == bpcisrc-addr.slot +apcisrc-addr.function == bpcisrc-addr.function) return 1; return 0; } @@ -15486,15 +15490,17 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, bool includeTypeInAddr) { virDomainHostdevSubsysUSBPtr usbsrc = def-source.subsys.u.usb; +virDomainHostdevSubsysPCIPtr pcisrc = def-source.subsys.u.pci; if (def-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI -def-source.subsys.u.pci.backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { -const char *backend = virDomainHostdevSubsysPCIBackendTypeToString(def-source.subsys.u.pci.backend); +pcisrc-backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { +const char *backend = +virDomainHostdevSubsysPCIBackendTypeToString(pcisrc-backend); if (!backend) { virReportError(VIR_ERR_INTERNAL_ERROR, _(unexpected pci hostdev driver name type %d), - def-source.subsys.u.pci.backend); +
[libvirt] [PATCH 5/8] Add virConnectPtr for qemuBuildSCSIHostdevDrvStr
Add a conn for future patches to be able to grab the secret when authenticating an iSCSI host device Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_command.c | 5 +++-- src/qemu/qemu_command.h | 5 +++-- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_hotplug.c | 34 -- src/qemu/qemu_hotplug.h | 9 ++--- 5 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dbdb871..1ab3ade 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5120,7 +5120,8 @@ qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev) } char * -qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, +qemuBuildSCSIHostdevDrvStr(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, qemuBuildCommandLineCallbacksPtr callbacks) { @@ -8866,7 +8867,7 @@ qemuBuildCommandLine(virConnectPtr conn, char *drvstr; virCommandAddArg(cmd, -drive); -if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps, callbacks))) +if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, qemuCaps, callbacks))) goto error; virCommandAddArg(cmd, drvstr); VIR_FREE(drvstr); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index cf51056..b71e964 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -170,10 +170,11 @@ char * qemuBuildUSBHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); -char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, +char * qemuBuildSCSIHostdevDrvStr(virConnectPtr conn, + virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps, qemuBuildCommandLineCallbacksPtr callbacks) -ATTRIBUTE_NONNULL(3); +ATTRIBUTE_NONNULL(4); char * qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ecccf6c..3c6fef7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6472,7 +6472,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break; case VIR_DOMAIN_DEVICE_HOSTDEV: -ret = qemuDomainAttachHostDevice(driver, vm, +ret = qemuDomainAttachHostDevice(dom-conn, driver, vm, dev-data.hostdev); if (!ret) dev-data.hostdev = NULL; @@ -6544,10 +6544,10 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, ret = qemuDomainDetachLease(driver, vm, dev-data.lease); break; case VIR_DOMAIN_DEVICE_NET: -ret = qemuDomainDetachNetDevice(driver, vm, dev); +ret = qemuDomainDetachNetDevice(dom-conn, driver, vm, dev); break; case VIR_DOMAIN_DEVICE_HOSTDEV: -ret = qemuDomainDetachHostDevice(driver, vm, dev); +ret = qemuDomainDetachHostDevice(dom-conn, driver, vm, dev); break; case VIR_DOMAIN_DEVICE_CHR: ret = qemuDomainDetachChrDevice(driver, vm, dev-data.chr); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7df5832..6773d50 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -860,7 +860,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, * netdev-specific code as appropriate), then also added to * the nets list (see cleanup:) if successful. */ -ret = qemuDomainAttachHostDevice(driver, vm, +ret = qemuDomainAttachHostDevice(conn, driver, vm, virDomainNetGetActualHostdev(net)); goto cleanup; } @@ -1547,7 +1547,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, } static int -qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver, +qemuDomainAttachHostSCSIDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { @@ -1594,7 +1595,7 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceHostdevAlias(vm-def, hostdev, -1) 0) goto cleanup; -if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv-qemuCaps, +if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, priv-qemuCaps, buildCommandLineCallbacks))) goto cleanup; @@ -1642,7 +1643,8 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver, return ret; } -int qemuDomainAttachHostDevice(virQEMUDriverPtr
[libvirt] [PATCH 8/8] hostdev: Add iSCSI hostdev XML
Introduce a new structure to handle an iSCSI host device based on the existing virDomainHostdevSubsysSCSI by adding a protocol='iscsi' to the source/ element. The hostdev structure mimics the existing disk/ element for an iSCSI device (network) device. New XML is: hostdev mode='subsystem' type='scsi' managed='yes' auth username='myname' secret type='iscsi' usage='mycluster_myname'/ /auth source protocol='iscsi' name='iqn.1992-01.com.example' host name='example.org' port='3260'/ /source address type='drive' controller='0' bus='0' target='2' unit='5'/ /hostdev The controller element will mimic the existing scsi_host code insomuch as when 'lsi' and 'virtio-scsi' are used. Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in | 142 --- docs/schemas/domaincommon.rng | 46 ++- src/conf/domain_conf.c | 152 ++--- .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args | 14 ++ .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml | 46 +++ .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args | 14 ++ .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml| 40 ++ ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args | 16 +++ ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml | 46 +++ .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args| 16 +++ .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml | 40 ++ tests/qemuxml2argvtest.c | 16 +++ tests/qemuxml2xmltest.c| 5 + 13 files changed, 524 insertions(+), 69 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b69da4c..82b9091 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2781,57 +2781,107 @@ lt;/devicesgt; .../pre + +por:/p + +pre + ... + lt;devicesgt; +lt;hostdev mode='subsystem' type='scsi'gt; + lt;source protocol='iscsi' name='iqn.2014-08.com.example:iscsi-nopool/1'gt; +lt;host name='example.com' port='3260'/gt; + lt;/sourcegt; + lt;auth username='myuser'gt; +lt;secret type='iscsi' usage='libvirtiscsi'/gt; + lt;/authgt; + lt;address type='drive' controller='0' bus='0' target='0' unit='0'/gt; +lt;/hostdevgt; + lt;/devicesgt; + .../pre + dl dtcodehostdev/code/dt ddThe codehostdev/code element is the main container for describing -host devices. For usb device passthrough codemode/code is always -subsystem and codetype/code is usb for a USB device, pci -for a PCI device and scsi for a SCSI device. When -codemanaged/code is yes for a PCI -device, it is detached from the host before being passed on to -the guest, and reattached to the host after the guest exits. -If codemanaged/code is omitted or no, and for USB -devices, the user is responsible to -call codevirNodeDeviceDettach/code (or codevirsh -nodedev-dettach/code) before starting the guest or -hot-plugging the device, -and codevirNodeDeviceReAttach/code (or codevirsh -nodedev-reattach/code) after hot-unplug or stopping the -guest. For SCSI device, user is responsible to make sure the device -is not used by host. -The optional codesgio/code (span class=sincesince 1.0.6/span) -attribute indicates whether the kernel will filter unprivileged -SG_IO commands for the disk, valid settings are filtered or -unfiltered. Defaults to filtered. +host devices. For each device, the codemode/code is always +subsystem and the codetype/code is one of the following values +with additional attributes noted. +dl + dtusb/dt + ddFor USB devices, the user is responsible to call +codevirNodeDeviceDettach/code (or +codevirsh nodedev-dettach/code) before starting the guest +or hot-plugging the device and codevirNodeDeviceReAttach/code +(or codevirsh nodedev-reattach/code) after hot-unplug or +stopping the guest. + /dd + dtpci/dt + ddFor PCI devices, when codemanaged/code is yes
[libvirt] [PATCH 6/8] hostdev: Introduce virDomainHostdevSubsysSCSIiSCSI
Create the structures and API's to hold and manage the iSCSI host device. This extends the 'scsi_host' definitions added in commit id '5c811dce'. A future patch will add the XML parsing, but that code requires some infrastructure to be in place first in order to handle the differences between a 'scsi_host' and an 'iSCSI host' device. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_audit.c | 20 +++- src/conf/domain_conf.c | 47 - src/conf/domain_conf.h | 20 src/qemu/qemu_cgroup.c | 35 --- src/qemu/qemu_command.c | 74 --- src/qemu/qemu_hotplug.c | 36 +-- src/security/security_apparmor.c | 6 ++ src/security/security_dac.c | 12 +++ src/security/security_selinux.c | 12 +++ src/util/virhostdev.c| 200 +-- 10 files changed, 349 insertions(+), 113 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index ee9baa2..3ad58b0 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -424,12 +424,22 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { -virDomainHostdevSubsysSCSIHostPtr scsihostsrc = scsisrc-u.host; -if (virAsprintfQuiet(address, %s:%d:%d:%d, - scsihostsrc-adapter, scsihostsrc-bus, - scsihostsrc-target, scsihostsrc-unit) 0) { -VIR_WARN(OOM while encoding audit message); +if (scsisrc-protocol == +VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { +/* Follow virDomainAuditDisk virDomainAuditGenericDev + * and don't audit the networked device. + */ goto cleanup; +} else { +virDomainHostdevSubsysSCSIHostPtr scsihostsrc = +scsisrc-u.host; +if (virAsprintfQuiet(address, %s:%d:%d:%d, + scsihostsrc-adapter, scsihostsrc-bus, + scsihostsrc-target, + scsihostsrc-unit) 0) { +VIR_WARN(OOM while encoding audit message); +goto cleanup; +} } break; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 55c0822..758b907 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1702,6 +1702,17 @@ virDomainHostdevDefPtr virDomainHostdevDefAlloc(void) return def; } +static void +virDomainHostdevSubsysSCSIiSCSIFree(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc) +{ +if (!iscsisrc) +return; +VIR_FREE(iscsisrc-path); +virStorageNetHostDefFree(iscsisrc-nhosts, iscsisrc-hosts); +virStorageAuthDefFree(iscsisrc-auth); +iscsisrc-auth = NULL; +} + void virDomainHostdevDefClear(virDomainHostdevDefPtr def) { if (!def) @@ -1732,8 +1743,15 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) } break; case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: -if (def-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) -VIR_FREE(def-source.subsys.u.scsi.u.host.adapter); +if (def-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { +virDomainHostdevSubsysSCSIPtr scsisrc = def-source.subsys.u.scsi; +if (scsisrc-protocol == +VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { +virDomainHostdevSubsysSCSIiSCSIFree(scsisrc-u.iscsi); +} else { +VIR_FREE(scsisrc-u.host.adapter); +} +} break; } } @@ -4313,8 +4331,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, } if (sgio) { -if (def-source.subsys.type != -VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { +if (def-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { virReportError(VIR_ERR_XML_ERROR, %s, _(sgio is only supported for scsi host device)); goto error; @@ -10161,6 +10178,22 @@ virDomainHostdevMatchSubsysSCSIHost(virDomainHostdevDefPtr a, } static int +virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr a, + virDomainHostdevDefPtr b) +{ +virDomainHostdevSubsysSCSIiSCSIPtr aiscsisrc = +a-source.subsys.u.scsi.u.iscsi; +virDomainHostdevSubsysSCSIiSCSIPtr biscsisrc = +b-source.subsys.u.scsi.u.iscsi; + +if (STREQ(aiscsisrc-hosts[0].name, biscsisrc-hosts[0].name) +STREQ(aiscsisrc-hosts[0].port, biscsisrc-hosts[0].port) +STREQ(aiscsisrc-path, biscsisrc-path)) +return 1; +return 0; +} + +static int virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
[libvirt] [PATCH 7/8] domain_conf: Common routine to handle network storage host xml def
In preparation for hostdev support for iSCSI and a virStorageNetHostDefPtr, split out the network disk storage parsing of the 'host' element into a separate routine. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 137 +++-- 1 file changed, 76 insertions(+), 61 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 758b907..814ab87 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4036,6 +4036,79 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, } static int +virDomainStorageHostParse(xmlNodePtr node, + virStorageNetHostDefPtr *hosts, + size_t *nhosts) +{ +int ret = -1; +xmlNodePtr child; +char *transport = NULL; +virStorageNetHostDef host; + +memset(host, 0, sizeof(host)); + +child = node-children; +while (child != NULL) { +if (child-type == XML_ELEMENT_NODE +xmlStrEqual(child-name, BAD_CAST host)) { + +host.transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + +/* transport can be tcp (default), unix or rdma. */ +if ((transport = virXMLPropString(child, transport))) { +host.transport = virStorageNetHostTransportTypeFromString(transport); +if (host.transport 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unknown protocol transport type '%s'), + transport); +goto cleanup; +} +} + +host.socket = virXMLPropString(child, socket); + +if (host.transport == VIR_STORAGE_NET_HOST_TRANS_UNIX +host.socket == NULL) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(missing socket for unix transport)); +goto cleanup; +} + +if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX +host.socket != NULL) { +virReportError(VIR_ERR_XML_ERROR, + _(transport '%s' does not support + socket attribute), + transport); +goto cleanup; +} + +VIR_FREE(transport); + +if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX) { +if (!(host.name = virXMLPropString(child, name))) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(missing name for host)); +goto cleanup; +} + +host.port = virXMLPropString(child, port); +} + +if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) 0) +goto cleanup; +} +child = child-next; +} +ret = 0; + + cleanup: +virStorageNetHostDefClear(host); +VIR_FREE(transport); +return ret; +} + +static int virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode, virDomainHostdevSubsysSCSIPtr scsisrc) { @@ -5014,13 +5087,8 @@ int virDomainDiskSourceParse(xmlNodePtr node, virStorageSourcePtr src) { -char *protocol = NULL; -char *transport = NULL; -virStorageNetHostDef host; -xmlNodePtr child; int ret = -1; - -memset(host, 0, sizeof(host)); +char *protocol = NULL; switch ((virStorageType)src-type) { case VIR_STORAGE_TYPE_FILE: @@ -5073,59 +5141,8 @@ virDomainDiskSourceParse(xmlNodePtr node, tmp[0] = '\0'; } -child = node-children; -while (child != NULL) { -if (child-type == XML_ELEMENT_NODE -xmlStrEqual(child-name, BAD_CAST host)) { - -host.transport = VIR_STORAGE_NET_HOST_TRANS_TCP; - -/* transport can be tcp (default), unix or rdma. */ -if ((transport = virXMLPropString(child, transport))) { -host.transport = virStorageNetHostTransportTypeFromString(transport); -if (host.transport 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(unknown protocol transport type '%s'), - transport); -goto cleanup; -} -} - -host.socket = virXMLPropString(child, socket); - -if (host.transport == VIR_STORAGE_NET_HOST_TRANS_UNIX -host.socket == NULL) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(missing socket for unix transport)); -goto cleanup; -} - -if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX -host.socket != NULL) { -
[libvirt] [PATCH 1/8] hostdev: Introduce virDomainHostdevSubsysUSB
Create a separate typedef for the hostdev union data describing USB. Then adjust the code to use the new pointer Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_audit.c | 4 ++-- src/conf/domain_conf.c | 50 +++- src/conf/domain_conf.h | 22 ++ src/lxc/lxc_cgroup.c | 4 ++-- src/lxc/lxc_controller.c | 10 src/lxc/lxc_driver.c | 16 ++--- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_command.c | 26 ++--- src/qemu/qemu_hotplug.c | 7 +++--- src/security/security_apparmor.c | 6 ++--- src/security/security_dac.c | 12 -- src/security/security_selinux.c | 10 src/security/virt-aa-helper.c| 5 ++-- src/util/virhostdev.c| 50 +++- 14 files changed, 104 insertions(+), 122 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index a3d6c67..8277b06 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -388,6 +388,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, char *address = NULL; char *device = NULL; const char *virt; +virDomainHostdevSubsysUSBPtr usbsrc = hostdev-source.subsys.u.usb; virUUIDFormat(vm-def-uuid, uuidstr); if (!(vmname = virAuditEncode(vm, vm-def-name))) { @@ -415,8 +416,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (virAsprintfQuiet(address, %.3d.%.3d, - hostdev-source.subsys.u.usb.bus, - hostdev-source.subsys.u.usb.device) 0) { + usbsrc-bus, usbsrc-device) 0) { VIR_WARN(OOM while encoding audit message); goto cleanup; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b91ccf7..046b4f8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3800,6 +3800,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, xmlNodePtr cur; char *startupPolicy = NULL; char *autoAddress; +virDomainHostdevSubsysUSBPtr usbsrc = def-source.subsys.u.usb; if ((startupPolicy = virXMLPropString(node, startupPolicy))) { def-startupPolicy = @@ -3816,7 +3817,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, if ((autoAddress = virXMLPropString(node, autoAddress))) { if (STREQ(autoAddress, yes)) -def-source.subsys.u.usb.autoAddress = true; +usbsrc-autoAddress = true; VIR_FREE(autoAddress); } @@ -3833,8 +3834,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, if (vendor) { got_vendor = true; -if (virStrToLong_ui(vendor, NULL, 0, -def-source.subsys.u.usb.vendor) 0) { +if (virStrToLong_ui(vendor, NULL, 0, usbsrc-vendor) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse vendor id %s), vendor); VIR_FREE(vendor); @@ -3852,7 +3852,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, if (product) { got_product = true; if (virStrToLong_ui(product, NULL, 0, -def-source.subsys.u.usb.product) 0) { +usbsrc-product) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse product %s), product); @@ -3870,8 +3870,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, bus = virXMLPropString(cur, bus); if (bus) { -if (virStrToLong_ui(bus, NULL, 0, -def-source.subsys.u.usb.bus) 0) { +if (virStrToLong_ui(bus, NULL, 0, usbsrc-bus) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse bus %s), bus); VIR_FREE(bus); @@ -3886,8 +3885,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, device = virXMLPropString(cur, device); if (device) { -if (virStrToLong_ui(device, NULL, 0, -def-source.subsys.u.usb.device) 0) { +if (virStrToLong_ui(device, NULL, 0, usbsrc-device) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot parse device %s), device); @@ -3910,7 +3908,7 @@
[libvirt] [PATCHv2 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
--- src/security/virt-aa-helper.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..c8f17f9 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1342,10 +1342,13 @@ main(int argc, char **argv) vah_info(include_file); vah_info(included_files); rc = 0; +} else if (ctl-def-virtType == VIR_DOMAIN_VIRT_LXC) { +rc = 0; } else if ((rc = update_include_file(include_file, included_files, - ctl-append)) != 0) + ctl-append)) != 0) { goto cleanup; +} /* create the profile from TEMPLATE */ -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] AppArmor lxc profile fixes
Diff to v1: * virt-aa-helper: don't return -1 when reloading the profile: nothing to do isn't a bad thing sometimes. Cédric Bosdonnat (2): Don't output libvirt-UUID.files for LXC apparmor profiles Rework lxc apparmor profile examples/apparmor/Makefile.am | 6 +- examples/apparmor/TEMPLATE.lxc| 15 examples/apparmor/{TEMPLATE = TEMPLATE.qemu} | 2 +- examples/apparmor/libvirt-lxc | 119 +++--- src/security/security_apparmor.c | 20 +++-- src/security/virt-aa-helper.c | 34 ++-- 6 files changed, 152 insertions(+), 44 deletions(-) create mode 100644 examples/apparmor/TEMPLATE.lxc rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%) -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] Rework lxc apparmor profile
Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default. This profile allows quite a lot, but strives to restrict access to dangerous resources. Removing the explicit authorizations to bash, systemd and cron files, forces them to keep the lxc profile for all applications inside the container. PUx permissions where leading to running systemd (and others tasks) unconfined. Put the generic files, network and capabilities restrictions directly in the TEMPLATE.lxc: this way, users can restrict them on a per container basis. --- examples/apparmor/Makefile.am | 6 +- examples/apparmor/TEMPLATE.lxc| 15 examples/apparmor/{TEMPLATE = TEMPLATE.qemu} | 2 +- examples/apparmor/libvirt-lxc | 119 +++--- src/security/security_apparmor.c | 20 +++-- src/security/virt-aa-helper.c | 29 +-- 6 files changed, 148 insertions(+), 43 deletions(-) create mode 100644 examples/apparmor/TEMPLATE.lxc rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%) diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am index a741e94..7a20e16 100644 --- a/examples/apparmor/Makefile.am +++ b/examples/apparmor/Makefile.am @@ -15,7 +15,8 @@ ## http://www.gnu.org/licenses/. EXTRA_DIST=\ - TEMPLATE\ + TEMPLATE.qemu \ + TEMPLATE.lxc\ libvirt-qemu\ libvirt-lxc \ usr.lib.libvirt.virt-aa-helper \ @@ -36,6 +37,7 @@ abstractions_DATA = \ templatesdir = $(apparmordir)/libvirt templates_DATA = \ - TEMPLATE \ + TEMPLATE.qemu \ + TEMPLATE.lxc \ $(NULL) endif WITH_APPARMOR_PROFILES diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc new file mode 100644 index 000..7b64885 --- /dev/null +++ b/examples/apparmor/TEMPLATE.lxc @@ -0,0 +1,15 @@ +# +# This profile is for the domain whose UUID matches this file. +# + +#include tunables/global + +profile LIBVIRT_TEMPLATE { + #include abstractions/libvirt-lxc + + # Globally allows everything to run under this profile + # These can be narrowed depending on the container's use. + file, + capability, + network, +} diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu similarity index 75% rename from examples/apparmor/TEMPLATE rename to examples/apparmor/TEMPLATE.qemu index 187dec5..008a221 100644 --- a/examples/apparmor/TEMPLATE +++ b/examples/apparmor/TEMPLATE.qemu @@ -5,5 +5,5 @@ #include tunables/global profile LIBVIRT_TEMPLATE { - #include abstractions/libvirt-driver + #include abstractions/libvirt-qemu } diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc index d404328..4bfb503 100644 --- a/examples/apparmor/libvirt-lxc +++ b/examples/apparmor/libvirt-lxc @@ -2,16 +2,115 @@ #include abstractions/base - # Needed for lxc-enter-namespace - capability sys_admin, - capability sys_chroot, + umount, - # Added for lxc-enter-namespace --cmd /bin/bash - /bin/bash PUx, + # ignore DENIED message on / remount + deny mount options=(ro, remount) - /, - /usr/sbin/cron PUx, - /usr/lib/systemd/systemd PUx, + # allow tmpfs mounts everywhere + mount fstype=tmpfs, - /usr/lib/libsystemd-*.so.* mr, - /usr/lib/libudev-*.so.* mr, - /etc/ld.so.cache mr, + # allow mqueue mounts everywhere + mount fstype=mqueue, + + # allow fuse mounts everywhere + mount fstype=fuse.*, + + # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted + mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/, + deny @{PROC}/sys/fs/** wklx, + + # allow efivars to be mounted, writing to it will be blocked though + mount fstype=efivarfs - /sys/firmware/efi/efivars/, + + # block some other dangerous paths + deny @{PROC}/sysrq-trigger rwklx, + deny @{PROC}/mem rwklx, + deny @{PROC}/kmem rwklx, + + # deny writes in /sys except for /sys/fs/cgroup, also allow + # fusectl, securityfs and debugfs to be mounted there (read-only) + mount fstype=fusectl - /sys/fs/fuse/connections/, + mount fstype=securityfs - /sys/kernel/security/, + mount fstype=debugfs - /sys/kernel/debug/, + mount fstype=proc - /proc/, + mount fstype=sysfs - /sys/, + deny /sys/firmware/efi/efivars/** rwklx, + deny /sys/kernel/security/** rwklx, + + # generated by: lxc-generate-aa-rules.py container-rules.base + deny /proc/sys/[^kn]*{,/**} wklx, + deny /proc/sys/k[^e]*{,/**} wklx, + deny /proc/sys/ke[^r]*{,/**} wklx, + deny /proc/sys/ker[^n]*{,/**} wklx, + deny /proc/sys/kern[^e]*{,/**} wklx, + deny /proc/sys/kerne[^l]*{,/**} wklx, + deny /proc/sys/kernel/[^smhd]*{,/**} wklx, + deny /proc/sys/kernel/d[^o]*{,/**} wklx, + deny /proc/sys/kernel/do[^m]*{,/**} wklx, + deny /proc/sys/kernel/dom[^a]*{,/**} wklx, + deny /proc/sys/kernel/doma[^i]*{,/**} wklx, + deny /proc/sys/kernel/domai[^n]*{,/**}
[libvirt] [PATCH] virseclabel: Resolve Coverity FORWARD_NULL issue
Resolve issue introduced by commit id '13adf1b' Signed-off-by: John Ferlan jfer...@redhat.com --- src/util/virseclabel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virseclabel.c b/src/util/virseclabel.c index e9d973c..02a8342 100644 --- a/src/util/virseclabel.c +++ b/src/util/virseclabel.c @@ -61,7 +61,7 @@ virSecurityLabelDefNew(const char *model) if (VIR_ALLOC(seclabel) 0 || VIR_STRDUP(seclabel-model, model) 0) { virSecurityLabelDefFree(seclabel); -seclabel = NULL; +return NULL; } seclabel-relabel = true; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libxl: Correct cast to virDomainDiskDiscard enum.
From: Ian Campbell ian.campb...@citrix.com This was converted to a typedef in 5a2bd4c9171d conf: more enum cleanups in src/conf/domain_conf.h causing: libxl/libxl_conf.c: In function 'libxlDiskSetDiscard': libxl/libxl_conf.c:724:19: error: conversion to incomplete type Signed-off-by: Ian Campbell ian.campb...@citrix.com --- This build failure was found by the osstest build bot http://lists.xen.org/archives/html/xen-devel/2014-07/msg01552.html Pushing under the build-breaker rule. src/libxl/libxl_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0b4a0b5..f620d47 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -721,7 +721,7 @@ libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard) if (!x_disk-readwrite) return 0; #if defined(LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE) -switch ((enum virDomainDiskDiscard)discard) { +switch ((virDomainDiskDiscard)discard) { case VIR_DOMAIN_DISK_DISCARD_DEFAULT: case VIR_DOMAIN_DISK_DISCARD_LAST: break; -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 00/16] Support for per-guest-node binding
On 08.07.2014 13:50, Martin Kletzander wrote: Currently we are only able to bind the whole domain to some host nodes using the /domain/numatune/memory element. Numerous requests were made to support host-guest numa node bindings, so this series tries to implement that using /domain/numatune/memnode elements. That is incompatible with automatic numa placement (numad) since that makes no sense. Some of these patches were ACK'd in the previous round, but this version completely rewrites the parsing and formatting of the numatune XML element and places it into a separate file. Hence the repost of all the patches even with those ACK'd ones. Patches 1-3 move some code around, patch 4 adds cell id specification into the XML (which is used later on). Then patches 5-7 rework the numatune handling, which clears out some odd things, but mostly cleans the parsing code. Patch 8 adds the support for memnode elements in the XML conf code and together with patch 9 enables the use of it outside numatune_conf.c. After that, I needed to probe qemu for 2 capabilities; for one of them patch 10 adds the possibility to probe for it, then two following patches 11 and 12 add the probing data. One of the capabilities tells us that we can use disjoint ranges (not necessarily for the cpus= param) with qemu, so patch 13 makes a use of it. Finally patch 14 uses the memnode data to tell qemu which host nodes should be used for the allocations of memory blocks. Patch 15 does almost nothing, but the next patch looks better with it. And the last patch, number 16, fixes a bug with KVM and cgroups. One last note, APIs for handling the memnode settings will be added later. I'm sending this to prepare the ground for Michal's work with hugepages. Martin Kletzander (16): qemu: purely a code movement qemu: remove useless error check conf: purely a code movement conf, schema: add 'id' field for cells numatune: create new module for numatune numatune: unify numatune struct and enum names numatune: Encapsulate numatune configuration in order to unify results conf, schema: add support for memnode elements numatune: add support for per-node memory bindings in private APIs qemu: allow qmp probing for cmdline options without params qemu: memory-backend-ram capability probing qemu: newer -numa parameter capability probing qemu: enable disjoint numa cpu ranges qemu: pass numa node binding preferences to qemu qemu: split out cpuset.mems setting qemu: leave restricting cpuset.mems after initialization docs/formatdomain.html.in | 26 +- docs/schemas/domaincommon.rng | 22 + po/POTFILES.in | 1 + src/Makefile.am| 3 +- src/conf/cpu_conf.c| 39 +- src/conf/cpu_conf.h| 3 +- src/conf/domain_conf.c | 203 ++- src/conf/domain_conf.h | 10 +- src/conf/numatune_conf.c | 589 + src/conf/numatune_conf.h | 108 src/libvirt_private.syms | 24 +- src/lxc/lxc_cgroup.c | 20 +- src/lxc/lxc_controller.c | 5 +- src/lxc/lxc_native.c | 15 +- src/parallels/parallels_driver.c | 7 +- src/qemu/qemu_capabilities.c | 16 +- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_cgroup.c | 52 +- src/qemu/qemu_cgroup.h | 4 +- src/qemu/qemu_command.c| 98 +++- src/qemu/qemu_driver.c | 84 ++- src/qemu/qemu_monitor.c| 6 +- src/qemu/qemu_monitor.h| 3 +- src/qemu/qemu_monitor_json.c | 8 +- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_process.c| 12 +- src/util/virnuma.c | 61 +-- src/util/virnuma.h | 28 +- tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 5 + tests/qemumonitorjsontest.c| 20 +- .../qemuxml2argv-cpu-numa-disjoint.args| 6 + .../qemuxml2argv-cpu-numa-disjoint.xml | 28 + tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 + .../qemuxml2argv-numatune-auto-prefer.xml | 29 + .../qemuxml2argv-numatune-memnode-no-memory.args | 8 + .../qemuxml2argv-numatune-memnode-no-memory.xml| 30 ++
Re: [libvirt] [PATCH v2 07/16] numatune: Encapsulate numatune configuration in order to unify results
On 08.07.2014 13:50, Martin Kletzander wrote: There were numerous places where numatune configuration (and thus domain config as well) was changed in different ways. On some places this even resulted in persistent domain definition not to be stable (it would change with daemon's restart). In order to uniformly change how numatune config is dealt with, all the internals are now accessible directly only in numatune_conf.c and outside this file accessors must be used. Signed-off-by: Martin Kletzander mklet...@redhat.com --- po/POTFILES.in | 1 + src/conf/domain_conf.c | 159 ++- src/conf/domain_conf.h | 8 +- src/conf/numatune_conf.c | 316 + src/conf/numatune_conf.h | 72 - src/libvirt_private.syms | 11 + src/lxc/lxc_cgroup.c | 19 +- src/lxc/lxc_controller.c | 5 +- src/lxc/lxc_native.c | 15 +- src/parallels/parallels_driver.c | 7 +- src/qemu/qemu_cgroup.c | 23 +- src/qemu/qemu_driver.c | 84 +++--- src/qemu/qemu_process.c| 8 +- src/util/virnuma.c | 48 ++-- src/util/virnuma.h | 2 +- .../qemuxml2argv-numatune-auto-prefer.xml | 29 ++ .../qemuxml2xmlout-numatune-auto-prefer.xml| 29 ++ tests/qemuxml2xmltest.c| 2 + 18 files changed, 553 insertions(+), 285 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml Nice. diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c index 14300f0..8b66558 100644 --- a/src/conf/numatune_conf.c +++ b/src/conf/numatune_conf.c +int +virDomainNumatuneParseXML(virDomainDefPtr def, + xmlXPathContextPtr ctxt) +{ +char *tmp = NULL; +int mode = -1; +int n = 0; +int placement = -1; +int ret = -1; +virBitmapPtr nodeset = NULL; +xmlNodePtr node = NULL; + +if (virXPathInt(count(./numatune), ctxt, n) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(cannot extract numatune nodes)); +goto cleanup; +} else if (n 1) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(only one numatune is supported)); +goto cleanup; +} + +node = virXPathNode(./numatune/memory[1], ctxt); + +if (def-numatune) { +virDomainNumatuneFree(def-numatune); +def-numatune = NULL; +} + +if (!node def-placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) +return 0; + +if (!node) { +/* We know that def-placement_mode is auto if we're here */ +if (virDomainNumatuneSet(def, VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO, + -1, NULL) 0) +goto cleanup; +return 0; +} + +tmp = virXMLPropString(node, mode); +if (tmp) { +mode = virDomainNumatuneMemModeTypeFromString(tmp); +if (mode 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Unsupported NUMA memory tuning mode '%s'), + tmp); +goto cleanup; +} +} +VIR_FREE(tmp); + +tmp = virXMLPropString(node, placement); +if (tmp) { +placement = virDomainNumatunePlacementTypeFromString(tmp); +if (placement 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Unsupported NUMA memory placement mode '%s'), + tmp); +goto cleanup; +} +} +VIR_FREE(tmp); + +tmp = virXMLPropString(node, nodeset); +if (tmp virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN) 0) +goto cleanup; +VIR_FREE(tmp); + +if (virDomainNumatuneSet(def, placement, mode, nodeset) 0) The virDomainNumatuneSet() takes a copy of @nodeset, so you need to call virBitmaskFree(nodeset); at the cleanup label. +goto cleanup; + +if (!n) { +ret = 0; +goto cleanup; +} + +ret = 0; + cleanup: +VIR_FREE(tmp); +return ret; +} + +int +virDomainNumatuneFormatXML(virBufferPtr buf, + virDomainNumatunePtr numatune) +{ +const char *tmp = NULL; s /const// .. + +if (!numatune) +return 0; + +virBufferAddLit(buf, numatune\n); +virBufferAdjustIndent(buf, 2); + +tmp = virDomainNumatuneMemModeTypeToString(numatune-memory.mode); +virBufferAsprintf(buf, memory mode='%s' , tmp); + +if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) { +
Re: [libvirt] [PATCH v2 08/16] conf, schema: add support for memnode elements
On 08.07.2014 13:50, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/formatdomain.html.in | 15 ++ docs/schemas/domaincommon.rng | 17 ++ src/conf/numatune_conf.c | 183 +++-- .../qemuxml2argv-numatune-memnode-no-memory.xml| 30 .../qemuxml2argv-numatune-memnode-nocpu.xml| 25 +++ .../qemuxml2argv-numatune-memnode.xml | 31 .../qemuxml2argv-numatune-memnodes-problematic.xml | 31 tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-numatune-memnode.xml| 33 tests/qemuxml2xmltest.c| 2 + 10 files changed, 356 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ad87b7c..d845c1b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -709,6 +709,8 @@ ... lt;numatunegt; lt;memory mode=strict nodeset=1-4,^3/gt; +lt;memnode cellid=0 mode=strict nodeset=1/gt; +lt;memnode cellid=2 mode=preferred nodeset=2/gt; lt;/numatunegt; ... lt;/domaingt; @@ -745,6 +747,19 @@ span class='since'Since 0.9.3/span /dd + dtcodememnode/code/dt + dd +Optional codememnode/code elements can specify memory allocation +policies per each guest NUMA node. For those nodes having no +corresponding codememnode/code element, the default from +element codememory/code will be used. Attribute codecellid/code +addresses guest NUMA node for which the settings are applied. +Attributes codemode/code and codenodeset/code have the same +meaning and syntax as in codememory/code element. + +This setting is not compatible with automatic placement. +span class='since'QEMU Since 1.2.6/span 1.2.8 actually + /dd /dl +static int +virDomainNumatuneNodeParseXML(virDomainDefPtr def, + xmlXPathContextPtr ctxt) +{ +int n = 0;; +int ret = -1; +size_t i = 0; +xmlNodePtr *nodes = NULL; + +if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes)) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Cannot extract memnode nodes)); +goto cleanup; +} + +if (!n) +return 0; + +if (def-numatune def-numatune-memory.specified +def-numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(Per-node binding is not compatible with + automatic NUMA placement.)); +goto cleanup; +} + +if (!def-cpu || !def-cpu-ncells) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Element 'memnode' is invalid without + any guest NUMA cells)); +goto cleanup; +} + +if (!def-numatune VIR_ALLOC(def-numatune) 0) +goto cleanup; Here you allow def-numatune to be allocated already. + +if (VIR_ALLOC_N(def-numatune-mem_nodes, def-cpu-ncells) 0) +goto cleanup; Which means, this can exists too. VIR_REALLOC_N() is safer IMO. + +def-numatune-nmem_nodes = def-cpu-ncells; + +for (i = 0; i n; i++) { +const char *tmp = NULL; No. s/const//. +int mode = 0; +unsigned int cellid = 0; +virDomainNumatuneNodePtr mem_node = NULL; +xmlNodePtr cur_node = nodes[i]; + +tmp = virXMLPropString(cur_node, cellid); +if (!tmp) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Missing required cellid attribute + in memnode element)); +goto cleanup; +} +if (virStrToLong_uip(tmp, NULL, 10, cellid) 0) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Invalid cellid attribute in memnode element)); Moreover, @tmp is leaked here. And it would be nice to tell users in the error message @tmp somehow. +goto cleanup; +} +VIR_FREE(tmp); + +if (cellid = def-numatune-nmem_nodes) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Argument 'cellid' in memnode element must + correspond to existing guest's
Re: [libvirt] [PATCH v2 04/16] conf, schema: add 'id' field for cells
On 08.07.2014 13:50, Martin Kletzander wrote: In XML format, by definition, order of fields should not matter, so order of parsing the elements doesn't affect the end result. When specifying guest NUMA cells, we depend only on the order of the 'cell' elements. With this patch all older domain XMLs are parsed as before, but with the 'id' attribute they are parsed and formatted according to that field. This will be useful when we have tuning settings for particular guest NUMA node. Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/formatdomain.html.in | 11 +++--- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c| 39 +++--- src/conf/cpu_conf.h| 3 +- src/qemu/qemu_command.c| 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 ++ tests/qemuxml2argvtest.c | 1 + .../qemuxml2xmlout-cpu-numa1.xml | 28 .../qemuxml2xmlout-cpu-numa2.xml | 28 tests/qemuxml2xmltest.c| 3 ++ 12 files changed, 139 insertions(+), 18 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b69da4c..ad87b7c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1030,8 +1030,8 @@ lt;cpugt; ... lt;numagt; - lt;cell cpus='0-3' memory='512000'/gt; - lt;cell cpus='4-7' memory='512000'/gt; + lt;cell id='0' cpus='0-3' memory='512000'/gt; + lt;cell id='1' cpus='4-7' memory='512000'/gt; lt;/numagt; ... lt;/cpugt; @@ -1041,8 +1041,11 @@ Each codecell/code element specifies a NUMA cell or a NUMA node. codecpus/code specifies the CPU or range of CPUs that are part of the node. codememory/code specifies the node memory in kibibytes - (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid - or nodeid in the increasing order starting from 0. + (i.e. blocks of 1024 bytes). All cells should have codeid/code + attribute in case referring to some cell is necessary in the code, + otherwise the cells are assigned ids in the increasing order starting + from 0. Mixing cells with and without the codeid/code attribute + is not recommended as it may result in unwanted behaviour. I'd note here, that the @id attribute is since 1.2.7 /p p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7be028d..155a33e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3902,6 +3902,11 @@ define name=numaCell element name=cell + optional +attribute name=id + ref name=unsignedInt/ +/attribute + /optional attribute name=cpus ref name=cpuset/ /attribute diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 811893d..9e0af08 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -152,7 +152,6 @@ virCPUDefCopy(const virCPUDef *cpu) copy-ncells_max = copy-ncells = cpu-ncells; for (i = 0; i cpu-ncells; i++) { -copy-cells[i].cellid = cpu-cells[i].cellid; copy-cells[i].mem = cpu-cells[i].mem; copy-cells[i].cpumask = virBitmapNewCopy(cpu-cells[i].cpumask); @@ -438,17 +437,46 @@ virCPUDefParseXML(xmlNodePtr node, for (i = 0; i n; i++) { char *cpus, *memory; int ret, ncpus = 0; +unsigned int cur_cell; +char *tmp = NULL; + +tmp = virXMLPropString(nodes[i], id); +if (!tmp) { +cur_cell = i; +} else { +ret = virStrToLong_ui(tmp, NULL, 10, cur_cell); +VIR_FREE(tmp); +if (ret == -1) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Invalid 'id' attribute in NUMA cell)); +goto error; +} +} If there's a typo in the @id, I think this can make users lives easier: diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 9e0af08..5003cf1 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -445,12 +445,14 @@ virCPUDefParseXML(xmlNodePtr node, cur_cell = i; } else { ret = virStrToLong_ui(tmp, NULL, 10, cur_cell);
Re: [libvirt] [PATCH v2 09/16] numatune: add support for per-node memory bindings in private APIs
On 08.07.2014 13:50, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/conf/numatune_conf.c | 111 --- src/conf/numatune_conf.h | 14 -- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 3 +- src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_driver.c | 10 ++--- src/util/virnuma.c | 6 +-- 7 files changed, 117 insertions(+), 30 deletions(-) diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c index 67fc799..60b6867 100644 --- a/src/conf/numatune_conf.c +++ b/src/conf/numatune_conf.c @@ -63,6 +63,18 @@ struct _virDomainNumatune { }; +static inline bool +numa_cell_specified(virDomainNumatunePtr numatune, Whoa, when I met this function call I thought to myself that this is some libnuma function. Please name it differently to match our virSomethingShiny pattern. +int cellid) +{ +if (numatune +cellid = 0 +cellid numatune-nmem_nodes) +return numatune-mem_nodes[cellid].nodeset; + +return false; +} + static int virDomainNumatuneNodeParseXML(virDomainDefPtr def, xmlXPathContextPtr ctxt) @@ -312,6 +324,7 @@ void virDomainNumatuneFree(virDomainNumatunePtr numatune) { size_t i = 0; + This change is spurious. Either move it to 8/16 or drop this one. if (!numatune) return; @@ -324,26 +337,37 @@ virDomainNumatuneFree(virDomainNumatunePtr numatune) } virDomainNumatuneMemMode -virDomainNumatuneGetMode(virDomainNumatunePtr numatune) +virDomainNumatuneGetMode(virDomainNumatunePtr numatune, + int cellid) { -return (numatune numatune-memory.specified) ? numatune-memory.mode : 0; +if (!numatune) +return 0; + +if (numa_cell_specified(numatune, cellid)) +return numatune-mem_nodes[cellid].mode; + +if (numatune-memory.specified) +return numatune-memory.mode; + +return 0; } virBitmapPtr virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune, -virBitmapPtr auto_nodeset) +virBitmapPtr auto_nodeset, +int cellid) { if (!numatune) return NULL; -if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) +if (numatune-memory.specified +numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) return auto_nodeset; -/* - * This weird logic has the same meaning as switch with - * auto/static/default, but can be more readably changed later. - */ -if (numatune-memory.placement != VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) +if (numa_cell_specified(numatune, cellid)) +return numatune-mem_nodes[cellid].nodeset; + +if (!numatune-memory.specified) return NULL; return numatune-memory.nodeset; @@ -351,23 +375,30 @@ virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune, char * virDomainNumatuneFormatNodeset(virDomainNumatunePtr numatune, - virBitmapPtr auto_nodeset) + virBitmapPtr auto_nodeset, + int cellid) { return virBitmapFormat(virDomainNumatuneGetNodeset(numatune, - auto_nodeset)); + auto_nodeset, + cellid)); } int virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune, virBitmapPtr auto_nodeset, -char **mask) +char **mask, +int cellid) { *mask = NULL; if (!numatune) return 0; -if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO +if (!numa_cell_specified(numatune, cellid) !numatune-memory.specified) +return 0; + +if (numatune-memory.specified +numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO !auto_nodeset) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Advice from numad is needed in case of @@ -375,7 +406,7 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune, return -1; } -*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset); +*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset, cellid); if (!*mask) return -1; @@ -469,6 +500,35 @@ virDomainNumatuneSet(virDomainDefPtr def, return ret; } +static bool +virDomainNumatuneNodesEqual(virDomainNumatunePtr n1, +virDomainNumatunePtr n2) +{ +size_t i = 0; + +if (n1-nmem_nodes != n2-nmem_nodes) +return false; + +for (i = 0; i n1-nmem_nodes; i++) { +
Re: [libvirt] [PATCH v2 14/16] qemu: pass numa node binding preferences to qemu
On 08.07.2014 13:50, Martin Kletzander wrote: Currently, we only bind the whole QEMU domain to memory nodes specified in nodemask altogether. That, however, doesn't make much sense when one wants to control from where the memory for particular guest nodes should be allocated. QEMU allows us to do that by specifying 'host-nodes' parameter for the 'memory-backend-ram' object, so let's use that. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_command.c| 59 +- .../qemuxml2argv-numatune-memnode-no-memory.args | 8 +++ .../qemuxml2argv-numatune-memnode.args | 11 .../qemuxml2argv-numatune-memnode.xml | 14 ++--- tests/qemuxml2argvtest.c | 7 +++ 5 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml index 18b00d8..49b328c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml @@ -1,12 +1,13 @@ domain type='qemu' nameQEMUGuest/name uuid9f4b6512-e73a-4a25-93e8-5307802821ce/uuid - memory unit='KiB'65536/memory - currentMemory unit='KiB'65536/currentMemory - vcpu placement='static'2/vcpu + memory unit='KiB'24682468/memory + currentMemory unit='KiB'24682468/currentMemory + vcpu placement='static'32/vcpu numatune -memory mode='strict' nodeset='0-3'/ +memory mode='strict' nodeset='0-7'/ memnode cellid='0' mode='preferred' nodeset='3'/ +memnode cellid='2' mode='strict' nodeset='1-2,5-7,^6'/ /numatune os type arch='x86_64' machine='pc'hvm/type @@ -14,8 +15,9 @@ /os cpu numa - cell id='0' cpus='0' memory='32768'/ - cell id='1' cpus='1' memory='32768'/ + cell id='0' cpus='0' memory='20002'/ + cell id='1' cpus='1-27,29' memory='660066'/ + cell id='2' cpus='28-31,^29' memory='24002400'/ AHA! This explain why I'm seeing the test error in 8/16. Something went wrong during the rebase I think. Because now I have to revert the squash in from 8/16 to make the test work again. Yes, it's failing now. /numa /cpu clock offset='utc'/ Unfortunately, I can't ACK neither this one (same as I couldn't 8/16). Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
Quoting Cédric Bosdonnat (cbosdon...@suse.com): --- src/security/virt-aa-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Hi, I'm acking this anyway bc I think you're right, but I'm trying to think of a case where this would still be useful. What if we want to allow only a certain container to have access to its cgroups, for instance, for nesting containers. Would virt-aa-helper and the .files be a way this would be done? I suppose we coudl always re-introduce this in that case... Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..d563b98 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1342,7 +1342,8 @@ main(int argc, char **argv) vah_info(include_file); vah_info(included_files); rc = 0; -} else if ((rc = update_include_file(include_file, +} else if (ctl-def-virtType != VIR_DOMAIN_VIRT_LXC + (rc = update_include_file(include_file, included_files, ctl-append)) != 0) goto cleanup; -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Rework lxc apparmor profile
Quoting Cédric Bosdonnat (cbosdon...@suse.com): Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default. This profile allows quite a lot, but strives to restrict access to dangerous resources. Removing the explicit authorizations to bash, systemd and cron files, forces them to keep the lxc profile for all applications inside the container. PUx permissions where leading to running systemd (and others tasks) unconfined. Put the generic files, network and capabilities restrictions directly in the TEMPLATE.lxc: this way, users can restrict them on a per container basis. --- examples/apparmor/Makefile.am | 6 +- examples/apparmor/TEMPLATE.lxc| 15 examples/apparmor/{TEMPLATE = TEMPLATE.qemu} | 2 +- examples/apparmor/libvirt-lxc | 119 +++--- src/security/security_apparmor.c | 20 +++-- src/security/virt-aa-helper.c | 29 +-- 6 files changed, 148 insertions(+), 43 deletions(-) create mode 100644 examples/apparmor/TEMPLATE.lxc rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%) diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am index a741e94..7a20e16 100644 --- a/examples/apparmor/Makefile.am +++ b/examples/apparmor/Makefile.am @@ -15,7 +15,8 @@ ## http://www.gnu.org/licenses/. EXTRA_DIST= \ - TEMPLATE\ + TEMPLATE.qemu \ + TEMPLATE.lxc\ libvirt-qemu\ libvirt-lxc \ usr.lib.libvirt.virt-aa-helper \ @@ -36,6 +37,7 @@ abstractions_DATA = \ templatesdir = $(apparmordir)/libvirt templates_DATA = \ - TEMPLATE \ + TEMPLATE.qemu \ + TEMPLATE.lxc \ $(NULL) endif WITH_APPARMOR_PROFILES diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc new file mode 100644 index 000..7b64885 --- /dev/null +++ b/examples/apparmor/TEMPLATE.lxc @@ -0,0 +1,15 @@ +# +# This profile is for the domain whose UUID matches this file. +# + +#include tunables/global + +profile LIBVIRT_TEMPLATE { + #include abstractions/libvirt-lxc + + # Globally allows everything to run under this profile + # These can be narrowed depending on the container's use. + file, + capability, + network, +} diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu similarity index 75% rename from examples/apparmor/TEMPLATE rename to examples/apparmor/TEMPLATE.qemu index 187dec5..008a221 100644 --- a/examples/apparmor/TEMPLATE +++ b/examples/apparmor/TEMPLATE.qemu @@ -5,5 +5,5 @@ #include tunables/global profile LIBVIRT_TEMPLATE { - #include abstractions/libvirt-driver + #include abstractions/libvirt-qemu } diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc index d404328..4bfb503 100644 --- a/examples/apparmor/libvirt-lxc +++ b/examples/apparmor/libvirt-lxc @@ -2,16 +2,115 @@ #include abstractions/base - # Needed for lxc-enter-namespace - capability sys_admin, - capability sys_chroot, + umount, - # Added for lxc-enter-namespace --cmd /bin/bash - /bin/bash PUx, + # ignore DENIED message on / remount + deny mount options=(ro, remount) - /, - /usr/sbin/cron PUx, - /usr/lib/systemd/systemd PUx, + # allow tmpfs mounts everywhere + mount fstype=tmpfs, - /usr/lib/libsystemd-*.so.* mr, - /usr/lib/libudev-*.so.* mr, - /etc/ld.so.cache mr, + # allow mqueue mounts everywhere + mount fstype=mqueue, + + # allow fuse mounts everywhere + mount fstype=fuse.*, + + # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted + mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/, + deny @{PROC}/sys/fs/** wklx, + + # allow efivars to be mounted, writing to it will be blocked though + mount fstype=efivarfs - /sys/firmware/efi/efivars/, + + # block some other dangerous paths + deny @{PROC}/sysrq-trigger rwklx, + deny @{PROC}/mem rwklx, + deny @{PROC}/kmem rwklx, + + # deny writes in /sys except for /sys/fs/cgroup, also allow + # fusectl, securityfs and debugfs to be mounted there (read-only) + mount fstype=fusectl - /sys/fs/fuse/connections/, + mount fstype=securityfs - /sys/kernel/security/, + mount fstype=debugfs - /sys/kernel/debug/, + mount fstype=proc - /proc/, + mount fstype=sysfs - /sys/, + deny /sys/firmware/efi/efivars/** rwklx, + deny /sys/kernel/security/** rwklx, + + # generated by: lxc-generate-aa-rules.py container-rules.base + deny /proc/sys/[^kn]*{,/**} wklx, + deny /proc/sys/k[^e]*{,/**} wklx, + deny /proc/sys/ke[^r]*{,/**} wklx, + deny /proc/sys/ker[^n]*{,/**} wklx, + deny /proc/sys/kern[^e]*{,/**} wklx, + deny /proc/sys/kerne[^l]*{,/**} wklx, + deny /proc/sys/kernel/[^smhd]*{,/**} wklx, + deny /proc/sys/kernel/d[^o]*{,/**} wklx, + deny
[libvirt] [PATCH] conf: Fix possible NULL dereference in virStorageVolTargetDefFormat
Commit dae1568c6c6455091e8cd9bc2e90a22af3d3880c converted the perms member of the virStorageVolTarget struct into a pointer to make it optional. But virStorageVolTargetDefFormat did not check perms for NULL before dereferencing it. --- src/conf/storage_conf.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9ac5975..aa29658 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1423,22 +1423,24 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferAsprintf(buf, format type='%s'/\n, format); } -virBufferAddLit(buf, permissions\n); -virBufferAdjustIndent(buf, 2); +if (def-perms) { +virBufferAddLit(buf, permissions\n); +virBufferAdjustIndent(buf, 2); -virBufferAsprintf(buf, mode0%o/mode\n, - def-perms-mode); -virBufferAsprintf(buf, owner%u/owner\n, - (unsigned int) def-perms-uid); -virBufferAsprintf(buf, group%u/group\n, - (unsigned int) def-perms-gid); +virBufferAsprintf(buf, mode0%o/mode\n, + def-perms-mode); +virBufferAsprintf(buf, owner%u/owner\n, + (unsigned int) def-perms-uid); +virBufferAsprintf(buf, group%u/group\n, + (unsigned int) def-perms-gid); -virBufferEscapeString(buf, label%s/label\n, - def-perms-label); +virBufferEscapeString(buf, label%s/label\n, + def-perms-label); -virBufferAdjustIndent(buf, -2); -virBufferAddLit(buf, /permissions\n); +virBufferAdjustIndent(buf, -2); +virBufferAddLit(buf, /permissions\n); +} if (def-timestamps) { virBufferAddLit(buf, timestamps\n); -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/2] doc: Improve snapshot/blockjob docs
On 07/11/2014 03:01 AM, Peter Krempa wrote: Peter Krempa (2): doc: Document that snapshot name of block-backed disk isnt autogenerated doc: Be more specific about semantics of _REUSE_EXT flag ACK series; you picked up on all my suggestions on v1. docs/formatsnapshot.html.in | 8 +--- src/libvirt.c | 23 ++- tools/virsh.pod | 21 + 3 files changed, 32 insertions(+), 20 deletions(-) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] virSecurityLabelDefParseXML: Rework
On 07/11/2014 03:32 AM, Michal Privoznik wrote: Instead of allocating the virSecurityLabelDef structure ourselves, we can utilize virSecurityLabelDefNew which even sets the default values for us. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 103 - 1 file changed, 50 insertions(+), 53 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7b90903..de60cd2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4553,91 +4553,87 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, unsigned int flags) { char *p; -virSecurityLabelDefPtr def = NULL; +virSecurityLabelDefPtr seclabel = NULL; Looks like you did a search-and-replace s/def/seclabel/... -if (VIR_ALLOC(def) 0) +p = virXPathStringLimit(string(./@model), +VIR_SECURITY_MODEL_BUFLEN - 1, ctxt); + +if (!(seclabel = virSecurityLabelDefNew(p))) goto error; +/* set seclabelault value */ ...but didn't pay attention to what got replaced :) -if (p != NULL) { +if (p) { if (STREQ(p, yes)) { -def-relabel = true; +seclabel-relabel = true; I'm guessing that this assignment is now redundant with the fact that it already defaults to this value; but I'm okay leaving it to make the code obvious. ACK with the typo fix. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/3] conf: Always format seclabel's model
On 07/11/2014 03:32 AM, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1113860 We've always done that. Well, until 990e46c45. Point is, if we don't format model, we may lose a domain on libvirtd restart. If the seclabel is implicit however, we should skip it's formatting. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 34 +++--- .../qemuxml2argv-seclabel-dynamic-none.xml | 28 ++ tests/qemuxml2xmltest.c| 1 + 3 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virSecurityLabelDef: use enum type for @type
On 07/11/2014 03:32 AM, Michal Privoznik wrote: There's this trend in libvirt of using enum types wherever possible. Now that I'm at virSecurityLabelDef let's rework @type item of the structure so we don't have to typecast it elsewhere. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 6 -- src/security/security_dac.c | 2 +- src/util/virseclabel.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) I'm not quite as sure about this one. This solves the issue of how to detect errors when parsing strings to enum, but required the use of an intermediate variable which in turn made the patch a net gain in lines of code. If someone forgets to use the intermediate variable for parsing, this backfires. On the other hand, parsing string to enum should be done in just one location, and that's the location touched by this patch. I'm 50-50 on whether to take this, so I'd like someone else to chime in with an opinion. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Fix possible NULL dereference in virStorageVolTargetDefFormat
On 07/11/2014 10:13 AM, Matthias Bolte wrote: Commit dae1568c6c6455091e8cd9bc2e90a22af3d3880c converted the perms member of the virStorageVolTarget struct into a pointer to make it optional. But virStorageVolTargetDefFormat did not check perms for NULL before dereferencing it. --- src/conf/storage_conf.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
On 07/11/2014 09:22 AM, Serge Hallyn wrote: Quoting Cédric Bosdonnat (cbosdon...@suse.com): --- src/security/virt-aa-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Hi, I'm acking this anyway bc I think you're right, but I'm trying to think of a case where this would still be useful. What if we want to allow only a certain container to have access to its cgroups, for instance, for nesting containers. Would virt-aa-helper and the .files be a way this would be done? I suppose we coudl always re-introduce this in that case... Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com I've pushed this one. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PREPOST 05/17] src/xenxs:Refactor PCI parsing code
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce the function xenParseXMPCI(.); which parses PCI config signed-off-by:David Kiarie davidkiar...@gmail.com --- src/xenxs/xen_xm.c | 192 +++-- 1 file changed, 99 insertions(+), 93 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index e30ab9c..7a5b47c 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -529,6 +529,102 @@ static int xenParseXMEventsActions(virConfPtr conf, virDomainDefPtr def) return 0; } +static int +xenParseXMPCI(virConfPtr conf, virDomainDefPtr def) +{ +virConfValuePtr list = virConfGetValue(conf, pci); +virDomainHostdevDefPtr hostdev = NULL; Blank line after local variables. +if (list list-type == VIR_CONF_LIST) { +list = list-list; +while (list) { +char domain[5]; +char bus[3]; +char slot[3]; +char func[2]; +char *key, *nextkey; +int domainID; +int busID; +int slotID; +int funcID; + +domain[0] = bus[0] = slot[0] = func[0] = '\0'; + +if ((list-type != VIR_CONF_STRING) || (list-str == NULL)) +goto skippci; + +/* pci=[':00:1b.0',':00:13.0'] */ +if (!(key = list-str)) +goto skippci; +if (!(nextkey = strchr(key, ':'))) +goto skippci; + +if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Domain %s too big for destination), key); Pre-existing, but while we are touching the code, I wonder if the use of virReportError here is correct? The device is skipped if there is a problem parsing it. I think these errors should be logged via VIR_WARN, but would like confirmation from another libvirt dev before asking you to change them. At the very least, the error should be changed to VIR_ERR_CONF_SYNTAX. Regards, Jim +goto skippci; +} + +key = nextkey + 1; +if (!(nextkey = strchr(key, ':'))) +goto skippci; + +if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Bus %s too big for destination), key); +goto skippci; +} + +key = nextkey + 1; +if (!(nextkey = strchr(key, '.'))) +goto skippci; + +if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Slot %s too big for destination), key); +goto skippci; +} + +key = nextkey + 1; +if (strlen(key) != 1) +goto skippci; + +if (virStrncpy(func, key, 1, sizeof(func)) == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Function %s too big for destination), key); +goto skippci; +} + +if (virStrToLong_i(domain, NULL, 16, domainID) 0) +goto skippci; +if (virStrToLong_i(bus, NULL, 16, busID) 0) +goto skippci; +if (virStrToLong_i(slot, NULL, 16, slotID) 0) +goto skippci; +if (virStrToLong_i(func, NULL, 16, funcID) 0) +goto skippci; + +if (!(hostdev = virDomainHostdevDefAlloc())) + return -1; + +hostdev-managed = false; +hostdev-source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; +hostdev-source.subsys.u.pci.addr.domain = domainID; +hostdev-source.subsys.u.pci.addr.bus = busID; +hostdev-source.subsys.u.pci.addr.slot = slotID; +hostdev-source.subsys.u.pci.addr.function = funcID; + +if (VIR_APPEND_ELEMENT(def-hostdevs, def-nhostdevs, hostdev) 0) { +virDomainHostdevDefFree(hostdev); +return -1; +} + +skippci: +list = list-next; +} +} + +return 0; +} virDomainDefPtr xenParseXM(virConfPtr conf, int xendConfigVersion, virCapsPtr caps) @@ -541,7 +637,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, virDomainDiskDefPtr disk = NULL; virDomainNetDefPtr net = NULL; virDomainGraphicsDefPtr graphics = NULL; -virDomainHostdevDefPtr hostdev = NULL; size_t i; unsigned long count; char *script = NULL; @@ -848,98 +943,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, goto cleanup; if
Re: [libvirt] [PREPOST 06/17] src/xenxs:Refactor code parsing CPU config
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce xenParseXMCPUFeatures(.); This function parses config related to CPU and power management signed-off-by:David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 116 - 1 file changed, 62 insertions(+), 54 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 7a5b47c..ebeeeb5 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -625,6 +625,66 @@ xenParseXMPCI(virConfPtr conf, virDomainDefPtr def) return 0; } +static int xenParseXMCPUFeatures(virConfPtr conf, virDomainDefPtr def) +{ +unsigned long count; +const char *str; +int val; The usual whitespace comments, but otherwise looks good. Regards, Jim +if (xenXMConfigGetULong(conf, vcpus, count, 1) 0 || +MAX_VIRT_CPUS count) +return -1; +def-maxvcpus = count; +if (xenXMConfigGetULong(conf, vcpu_avail, count, -1) 0) +return -1; +def-vcpus = MIN(count_one_bits_l(count), def-maxvcpus); + +if (xenXMConfigGetString(conf, cpus, str, NULL) 0) +return -1; +if (str (virBitmapParse(str, 0, def-cpumask, 4096) 0)) +return -1; + +if (STREQ(def-os.type, hvm)) { +if (xenXMConfigGetBool(conf, pae, val, 0) 0) +return -1; +else if (val) +def-features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON; +if (xenXMConfigGetBool(conf, acpi, val, 0) 0) +return -1; +else if (val) +def-features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON; +if (xenXMConfigGetBool(conf, apic, val, 0) 0) +return -1; +else if (val) +def-features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON; +if (xenXMConfigGetBool(conf, hap, val, 0) 0) +return -1; +else if (val) +def-features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON; +if (xenXMConfigGetBool(conf, viridian, val, 0) 0) +return -1; +else if (val) +def-features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON; + +if (xenXMConfigGetBool(conf, hpet, val, -1) 0) +return -1; +else if (val != -1) { +virDomainTimerDefPtr timer; + +if (VIR_ALLOC_N(def-clock.timers, 1) 0 || +VIR_ALLOC(timer) 0) +return -1; + +timer-name = VIR_DOMAIN_TIMER_NAME_HPET; +timer-present = val; +timer-tickpolicy = -1; + +def-clock.ntimers = 1; +def-clock.timers[0] = timer; +} +} + +return 0; +} virDomainDefPtr xenParseXM(virConfPtr conf, int xendConfigVersion, virCapsPtr caps) @@ -638,7 +698,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, virDomainNetDefPtr net = NULL; virDomainGraphicsDefPtr graphics = NULL; size_t i; -unsigned long count; char *script = NULL; char *listenAddr = NULL; @@ -703,59 +762,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, } if (xenParseXMMem(conf, def) 0) goto cleanup; - -if (xenXMConfigGetULong(conf, vcpus, count, 1) 0 || -MAX_VIRT_CPUS count) -goto cleanup; -def-maxvcpus = count; -if (xenXMConfigGetULong(conf, vcpu_avail, count, -1) 0) -goto cleanup; -def-vcpus = MIN(count_one_bits_l(count), def-maxvcpus); - -if (xenXMConfigGetString(conf, cpus, str, NULL) 0) -goto cleanup; -if (str (virBitmapParse(str, 0, def-cpumask, 4096) 0)) -goto cleanup; - -if (hvm) { -if (xenXMConfigGetBool(conf, pae, val, 0) 0) -goto cleanup; -else if (val) -def-features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON; -if (xenXMConfigGetBool(conf, acpi, val, 0) 0) -goto cleanup; -else if (val) -def-features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON; -if (xenXMConfigGetBool(conf, apic, val, 0) 0) -goto cleanup; -else if (val) -def-features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON; -if (xenXMConfigGetBool(conf, hap, val, 0) 0) -goto cleanup; -else if (val) -def-features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON; -if (xenXMConfigGetBool(conf, viridian, val, 0) 0) -goto cleanup; -else if (val) -def-features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON; - -if (xenXMConfigGetBool(conf, hpet, val, -1) 0) -goto cleanup; -else if (val != -1) { -virDomainTimerDefPtr timer; - -if
[libvirt] [PATCH v2] support for QEMU vhost-user
This patch adds support for the QEMU vhost-user feature to libvirt. vhost-user enables the communication between a QEMU virtual machine and other userspace process using the Virtio transport protocol. It uses a char dev (e.g. Unix socket) for the control plane, while the data plane based on shared memory. The XML looks like: interface type='vhostuser' source type='unix' path='/tmp/vhost.sock' mode='server'/ mac address='52:54:00:3b:83:1a'/ model type='virtio'/ /interface changes from v1: * addressed comments * removed unnecessary checks * series merged in a single patch The previous version of this patch can be found at: http://www.redhat.com/archives/libvir-list/2014-July/msg00111.html Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- docs/formatdomain.html.in | 34 + docs/schemas/domaincommon.rng | 25 +++ src/conf/domain_conf.c | 87 ++ src/conf/domain_conf.h | 10 ++- src/libxl/libxl_conf.c | 1 + src/lxc/lxc_process.c | 1 + src/qemu/qemu_command.c| 63 src/uml/uml_conf.c | 5 ++ src/xenxs/xen_sxpr.c | 1 + .../qemuxml2argv-net-vhostuser.args| 7 ++ .../qemuxml2argv-net-vhostuser.xml | 33 tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c| 1 + 13 files changed, 267 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3f8bbee..606b7d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3927,6 +3927,40 @@ qemu-kvm -net nic,model=? /dev/null span class=sinceSince 0.9.5/span /p +h5a name=elementVhostuservhost-user interface/a/h5 + +p + vhost-user enables the communication between a QEMU virtual machine + and other userspace process using the Virtio transport protocol. + A char dev (e.g. Unix socket) is used for the control plane, while + the data plane is based on shared memory. +/p + +pre + ... + lt;devicesgt; +lt;interface type='vhostuser'gt; + lt;source type='unix' path='/tmp/vhost.sock' mode='server'gt; + lt;/sourcegt; + lt;mac address='52:54:00:3b:83:1a'gt; + lt;/macgt; + lt;model type='virtio'gt; + lt;/modelgt; +lt;/interfacegt; + lt;/devicesgt; + .../pre + +p + The codelt;sourcegt;/code element has to be specified + along with the type of char device. + Currently, only type='unix' is supported, where the path (the + directory path of the socket) and mode attributes are required. + Both codemode='server'/code and codemode='client'/code + are supported. + vhost-user requires the virtio model type, thus the + codelt;modelgt;/code element is mandatory. +/p + h4a name=elementsInputInput devices/a/h4 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af51eee..c9c02b6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1968,6 +1968,31 @@ /group group attribute name=type +valuevhostuser/value + /attribute + interleave + element name=source +attribute name=type + choice +valueunix/value + /choice +/attribute +attribute name=path + ref name=absFilePath/ +/attribute +attribute name=mode + choice +valueserver/value +valueclient/value + /choice +/attribute +empty/ + /element +ref name=interface-options/ + /interleave +/group +group + attribute name=type valuenetwork/value /attribute interleave diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8df43b7..fb286c6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy, VIR_DOMAIN_FS_WRPOLICY_LAST, VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, user, ethernet, + vhostuser, server, client, mcast, @@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def-data.ethernet.ipaddr); break; +case VIR_DOMAIN_NET_TYPE_VHOSTUSER: +
Re: [libvirt] [PATCH 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
On Fri, 2014-07-11 at 11:03 -0600, Eric Blake wrote: On 07/11/2014 09:22 AM, Serge Hallyn wrote: Quoting Cédric Bosdonnat (cbosdon...@suse.com): --- src/security/virt-aa-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Hi, I'm acking this anyway bc I think you're right, but I'm trying to think of a case where this would still be useful. What if we want to allow only a certain container to have access to its cgroups, for instance, for nesting containers. Would virt-aa-helper and the .files be a way this would be done? I suppose we coudl always re-introduce this in that case... Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com I've pushed this one. Huh, I found a regression with this one... sent a v2 earlier today. -- Cedric -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Rework lxc apparmor profile
Quoting Cedric Bosdonnat (cbosdon...@suse.com): On Fri, 2014-07-11 at 16:08 +, Serge Hallyn wrote: Quoting Cédric Bosdonnat (cbosdon...@suse.com): Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default. This profile allows quite a lot, but strives to restrict access to dangerous resources. Removing the explicit authorizations to bash, systemd and cron files, forces them to keep the lxc profile for all applications inside the container. PUx permissions where leading to running systemd (and others tasks) unconfined. Put the generic files, network and capabilities restrictions directly in the TEMPLATE.lxc: this way, users can restrict them on a per container basis. --- examples/apparmor/Makefile.am | 6 +- examples/apparmor/TEMPLATE.lxc| 15 examples/apparmor/{TEMPLATE = TEMPLATE.qemu} | 2 +- examples/apparmor/libvirt-lxc | 119 +++--- src/security/security_apparmor.c | 20 +++-- src/security/virt-aa-helper.c | 29 +-- 6 files changed, 148 insertions(+), 43 deletions(-) create mode 100644 examples/apparmor/TEMPLATE.lxc rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%) diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am index a741e94..7a20e16 100644 --- a/examples/apparmor/Makefile.am +++ b/examples/apparmor/Makefile.am @@ -15,7 +15,8 @@ ## http://www.gnu.org/licenses/. EXTRA_DIST= \ - TEMPLATE\ + TEMPLATE.qemu \ + TEMPLATE.lxc\ libvirt-qemu\ libvirt-lxc \ usr.lib.libvirt.virt-aa-helper \ @@ -36,6 +37,7 @@ abstractions_DATA = \ templatesdir = $(apparmordir)/libvirt templates_DATA = \ - TEMPLATE \ + TEMPLATE.qemu \ + TEMPLATE.lxc \ $(NULL) endif WITH_APPARMOR_PROFILES diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc new file mode 100644 index 000..7b64885 --- /dev/null +++ b/examples/apparmor/TEMPLATE.lxc @@ -0,0 +1,15 @@ +# +# This profile is for the domain whose UUID matches this file. +# + +#include tunables/global + +profile LIBVIRT_TEMPLATE { + #include abstractions/libvirt-lxc + + # Globally allows everything to run under this profile + # These can be narrowed depending on the container's use. + file, + capability, + network, +} diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu similarity index 75% rename from examples/apparmor/TEMPLATE rename to examples/apparmor/TEMPLATE.qemu index 187dec5..008a221 100644 --- a/examples/apparmor/TEMPLATE +++ b/examples/apparmor/TEMPLATE.qemu @@ -5,5 +5,5 @@ #include tunables/global profile LIBVIRT_TEMPLATE { - #include abstractions/libvirt-driver + #include abstractions/libvirt-qemu } diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc index d404328..4bfb503 100644 --- a/examples/apparmor/libvirt-lxc +++ b/examples/apparmor/libvirt-lxc @@ -2,16 +2,115 @@ #include abstractions/base - # Needed for lxc-enter-namespace - capability sys_admin, - capability sys_chroot, + umount, - # Added for lxc-enter-namespace --cmd /bin/bash - /bin/bash PUx, + # ignore DENIED message on / remount + deny mount options=(ro, remount) - /, - /usr/sbin/cron PUx, - /usr/lib/systemd/systemd PUx, + # allow tmpfs mounts everywhere + mount fstype=tmpfs, - /usr/lib/libsystemd-*.so.* mr, - /usr/lib/libudev-*.so.* mr, - /etc/ld.so.cache mr, + # allow mqueue mounts everywhere + mount fstype=mqueue, + + # allow fuse mounts everywhere + mount fstype=fuse.*, + + # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted + mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/, + deny @{PROC}/sys/fs/** wklx, + + # allow efivars to be mounted, writing to it will be blocked though + mount fstype=efivarfs - /sys/firmware/efi/efivars/, + + # block some other dangerous paths + deny @{PROC}/sysrq-trigger rwklx, + deny @{PROC}/mem rwklx, + deny @{PROC}/kmem rwklx, + + # deny writes in /sys except for /sys/fs/cgroup, also allow + # fusectl, securityfs and debugfs to be mounted there (read-only) + mount fstype=fusectl - /sys/fs/fuse/connections/, + mount fstype=securityfs - /sys/kernel/security/, + mount fstype=debugfs - /sys/kernel/debug/, + mount fstype=proc - /proc/, + mount fstype=sysfs - /sys/, + deny /sys/firmware/efi/efivars/** rwklx, + deny /sys/kernel/security/** rwklx, + + # generated by: lxc-generate-aa-rules.py
Re: [libvirt] [PATCH 2/2] Rework lxc apparmor profile
On Fri, 2014-07-11 at 16:08 +, Serge Hallyn wrote: Quoting Cédric Bosdonnat (cbosdon...@suse.com): Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default. This profile allows quite a lot, but strives to restrict access to dangerous resources. Removing the explicit authorizations to bash, systemd and cron files, forces them to keep the lxc profile for all applications inside the container. PUx permissions where leading to running systemd (and others tasks) unconfined. Put the generic files, network and capabilities restrictions directly in the TEMPLATE.lxc: this way, users can restrict them on a per container basis. --- examples/apparmor/Makefile.am | 6 +- examples/apparmor/TEMPLATE.lxc| 15 examples/apparmor/{TEMPLATE = TEMPLATE.qemu} | 2 +- examples/apparmor/libvirt-lxc | 119 +++--- src/security/security_apparmor.c | 20 +++-- src/security/virt-aa-helper.c | 29 +-- 6 files changed, 148 insertions(+), 43 deletions(-) create mode 100644 examples/apparmor/TEMPLATE.lxc rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%) diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am index a741e94..7a20e16 100644 --- a/examples/apparmor/Makefile.am +++ b/examples/apparmor/Makefile.am @@ -15,7 +15,8 @@ ## http://www.gnu.org/licenses/. EXTRA_DIST=\ - TEMPLATE\ + TEMPLATE.qemu \ + TEMPLATE.lxc\ libvirt-qemu\ libvirt-lxc \ usr.lib.libvirt.virt-aa-helper \ @@ -36,6 +37,7 @@ abstractions_DATA = \ templatesdir = $(apparmordir)/libvirt templates_DATA = \ - TEMPLATE \ + TEMPLATE.qemu \ + TEMPLATE.lxc \ $(NULL) endif WITH_APPARMOR_PROFILES diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc new file mode 100644 index 000..7b64885 --- /dev/null +++ b/examples/apparmor/TEMPLATE.lxc @@ -0,0 +1,15 @@ +# +# This profile is for the domain whose UUID matches this file. +# + +#include tunables/global + +profile LIBVIRT_TEMPLATE { + #include abstractions/libvirt-lxc + + # Globally allows everything to run under this profile + # These can be narrowed depending on the container's use. + file, + capability, + network, +} diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu similarity index 75% rename from examples/apparmor/TEMPLATE rename to examples/apparmor/TEMPLATE.qemu index 187dec5..008a221 100644 --- a/examples/apparmor/TEMPLATE +++ b/examples/apparmor/TEMPLATE.qemu @@ -5,5 +5,5 @@ #include tunables/global profile LIBVIRT_TEMPLATE { - #include abstractions/libvirt-driver + #include abstractions/libvirt-qemu } diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc index d404328..4bfb503 100644 --- a/examples/apparmor/libvirt-lxc +++ b/examples/apparmor/libvirt-lxc @@ -2,16 +2,115 @@ #include abstractions/base - # Needed for lxc-enter-namespace - capability sys_admin, - capability sys_chroot, + umount, - # Added for lxc-enter-namespace --cmd /bin/bash - /bin/bash PUx, + # ignore DENIED message on / remount + deny mount options=(ro, remount) - /, - /usr/sbin/cron PUx, - /usr/lib/systemd/systemd PUx, + # allow tmpfs mounts everywhere + mount fstype=tmpfs, - /usr/lib/libsystemd-*.so.* mr, - /usr/lib/libudev-*.so.* mr, - /etc/ld.so.cache mr, + # allow mqueue mounts everywhere + mount fstype=mqueue, + + # allow fuse mounts everywhere + mount fstype=fuse.*, + + # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted + mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/, + deny @{PROC}/sys/fs/** wklx, + + # allow efivars to be mounted, writing to it will be blocked though + mount fstype=efivarfs - /sys/firmware/efi/efivars/, + + # block some other dangerous paths + deny @{PROC}/sysrq-trigger rwklx, + deny @{PROC}/mem rwklx, + deny @{PROC}/kmem rwklx, + + # deny writes in /sys except for /sys/fs/cgroup, also allow + # fusectl, securityfs and debugfs to be mounted there (read-only) + mount fstype=fusectl - /sys/fs/fuse/connections/, + mount fstype=securityfs - /sys/kernel/security/, + mount fstype=debugfs - /sys/kernel/debug/, + mount fstype=proc - /proc/, + mount fstype=sysfs - /sys/, + deny /sys/firmware/efi/efivars/** rwklx, + deny /sys/kernel/security/** rwklx, + + # generated by: lxc-generate-aa-rules.py container-rules.base + deny /proc/sys/[^kn]*{,/**} wklx, + deny /proc/sys/k[^e]*{,/**} wklx, + deny /proc/sys/ke[^r]*{,/**} wklx, + deny /proc/sys/ker[^n]*{,/**} wklx,
Re: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce the function xenParseXMDisk(..); Parses disk config signed-off-by: David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 204 - 1 file changed, 108 insertions(+), 96 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index ebeeeb5..80a7941 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -685,87 +685,14 @@ static int xenParseXMCPUFeatures(virConfPtr conf, virDomainDefPtr def) return 0; } -virDomainDefPtr -xenParseXM(virConfPtr conf, int xendConfigVersion, - virCapsPtr caps) +static int xenParseXMDisk(virConfPtr conf, virDomainDefPtr def, + int xendConfigVersion) { -const char *str; -int hvm = 0; -int val; -virConfValuePtr list; -virDomainDefPtr def = NULL; +const char *str = NULL; virDomainDiskDefPtr disk = NULL; -virDomainNetDefPtr net = NULL; -virDomainGraphicsDefPtr graphics = NULL; -size_t i; -char *script = NULL; -char *listenAddr = NULL; - -if (VIR_ALLOC(def) 0) -return NULL; - -def-virtType = VIR_DOMAIN_VIRT_XEN; -def-id = -1; -if (xenParseXMGeneral(conf, def, caps) 0) -goto cleanup; -hvm = (STREQ(def-os.type, hvm)); -if (hvm) { -const char *boot; -if (xenXMConfigCopyString(conf, kernel, def-os.loader) 0) -goto cleanup; - -if (xenXMConfigGetString(conf, boot, boot, c) 0) -goto cleanup; +int hvm = STREQ(def-os.type, hvm); +virConfValuePtr list = virConfGetValue(conf, disk); -for (i = 0; i VIR_DOMAIN_BOOT_LAST boot[i]; i++) { -switch (*boot) { -case 'a': -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY; -break; -case 'd': -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM; -break; -case 'n': -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_NET; -break; -case 'c': -default: -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK; -break; -} -def-os.nBootDevs++; -} -} else { -const char *extra, *root; - -if (xenXMConfigCopyStringOpt(conf, bootloader, def-os.bootloader) 0) -goto cleanup; -if (xenXMConfigCopyStringOpt(conf, bootargs, def-os.bootloaderArgs) 0) -goto cleanup; - -if (xenXMConfigCopyStringOpt(conf, kernel, def-os.kernel) 0) -goto cleanup; -if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd) 0) -goto cleanup; -if (xenXMConfigGetString(conf, extra, extra, NULL) 0) -goto cleanup; -if (xenXMConfigGetString(conf, root, root, NULL) 0) -goto cleanup; - -if (root) { -if (virAsprintf(def-os.cmdline, root=%s %s, root, extra) 0) -goto cleanup; -} else { -if (VIR_STRDUP(def-os.cmdline, extra) 0) -goto cleanup; -} -} -if (xenParseXMMem(conf, def) 0) -goto cleanup; -if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator) 0) -goto cleanup; - -list = virConfGetValue(conf, disk); if (list list-type == VIR_CONF_LIST) { list = list-list; while (list) { @@ -779,7 +706,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, head = list-str; if (!(disk = virDomainDiskDefNew())) -goto cleanup; +return -1; /* * Disks have 3 components, SOURCE,DEST-DEVICE,MODE @@ -798,10 +725,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, ignore_value(virDomainDiskSetSource(disk, NULL)); } else { if (VIR_STRNDUP(tmp, head, offset - head) 0) -goto cleanup; +return -1; if (virDomainDiskSetSource(disk, tmp) 0) { VIR_FREE(tmp); -goto cleanup; +return -1; } VIR_FREE(tmp); } @@ -815,12 +742,12 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (!(offset = strchr(head, ','))) goto skipdisk; if (VIR_ALLOC_N(disk-dst, (offset - head) + 1) 0) -goto cleanup; +return -1; if (virStrncpy(disk-dst, head, offset - head, (offset - head) + 1) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Dest file %s too big for destination), head); -goto cleanup; +
Re: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code
On 07/11/2014 01:04 PM, Jim Fehlig wrote: David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce the function xenParseXMDisk(..); Parses disk config signed-off-by: David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 204 - 1 file changed, 108 insertions(+), 96 deletions(-) @@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, /* Maintain list in sorted order according to target device name */ if (VIR_APPEND_ELEMENT(def-disks, def-ndisks, disk) 0) -goto cleanup; +return -1; skipdisk: list = list-next; The next line here is virDomainDiskDefFree(disk); which is not right in the case VIR_APPEND_ELEMENT succeeds (pre-existing bug). 'disk' should be set to NULL if VIR_APPEND_ELEMENT succeeds, so we don't free the disk we've just added to the disk list. VIR_APPEND_ELEMENT(, , disk) sets disk=NULL on success, precisely so you don't have to worry about double-freeing it (if you call the macro, you are guaranteed either transfer semantics and success, or no change on failure). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
On 07/11/2014 07:01 AM, Cédric Bosdonnat wrote: --- src/security/virt-aa-helper.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..c8f17f9 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1342,10 +1342,13 @@ main(int argc, char **argv) vah_info(include_file); vah_info(included_files); rc = 0; +} else if (ctl-def-virtType == VIR_DOMAIN_VIRT_LXC) { +rc = 0; } else if ((rc = update_include_file(include_file, included_files, - ctl-append)) != 0) + ctl-append)) != 0) { goto cleanup; +} I squashed this on top of a revert of v1, since I had pushed that before realizing you had posted a v2, and pushed the result. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virseclabel: Resolve Coverity FORWARD_NULL issue
On 07/11/2014 07:42 AM, John Ferlan wrote: Resolve issue introduced by commit id '13adf1b' Signed-off-by: John Ferlan jfer...@redhat.com --- src/util/virseclabel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK diff --git a/src/util/virseclabel.c b/src/util/virseclabel.c index e9d973c..02a8342 100644 --- a/src/util/virseclabel.c +++ b/src/util/virseclabel.c @@ -61,7 +61,7 @@ virSecurityLabelDefNew(const char *model) if (VIR_ALLOC(seclabel) 0 || VIR_STRDUP(seclabel-model, model) 0) { virSecurityLabelDefFree(seclabel); -seclabel = NULL; +return NULL; } seclabel-relabel = true; -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Fix possible NULL dereference in virStorageVolTargetDefFormat
On 07/11/2014 11:00 AM, Eric Blake wrote: On 07/11/2014 10:13 AM, Matthias Bolte wrote: Commit dae1568c6c6455091e8cd9bc2e90a22af3d3880c converted the perms member of the virStorageVolTarget struct into a pointer to make it optional. But virStorageVolTargetDefFormat did not check perms for NULL before dereferencing it. --- src/conf/storage_conf.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) ACK. Now pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2]util:openvswitch:Delete port if it is exist when add port
If the ovs service stop abnormal, or host cold reboot, vm is destroyed after ovs service stop. The ovs port which connect to interface of vm will not be clear. When the ovs service restart, recover configuration from db, but the interface is no exist, port recovery failed, and then vm restart on the same host, libvirt add port again, but the port configuration is same as before, ovs will not connect the interface, only store the configuration in db. Below will trigger this problem, virsh start vm service openvswitch-switch stop virsh destroy vm service openvswitch-switch start virsh start vm Signed-off-by: Chunhe Li lichu...@huawei.com --- src/util/virnetdevopenvswitch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 9bcbfb1..2c414ad 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -84,8 +84,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, cmd = virCommandNew(OVSVSCTL); -virCommandAddArgList(cmd, --timeout=5, --, --may-exist, add-port, -brname, ifname, NULL); +virCommandAddArgList(cmd, --timeout=5, --, --if-exists, del-port, +ifname, --, add-port, brname, ifname, NULL); if (virtVlan virtVlan-nTags 0) { -- 1.9.2.msysgit.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code
Eric Blake wrote: On 07/11/2014 01:04 PM, Jim Fehlig wrote: David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce the function xenParseXMDisk(..); Parses disk config signed-off-by: David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 204 - 1 file changed, 108 insertions(+), 96 deletions(-) @@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, /* Maintain list in sorted order according to target device name */ if (VIR_APPEND_ELEMENT(def-disks, def-ndisks, disk) 0) -goto cleanup; +return -1; skipdisk: list = list-next; The next line here is virDomainDiskDefFree(disk); which is not right in the case VIR_APPEND_ELEMENT succeeds (pre-existing bug). 'disk' should be set to NULL if VIR_APPEND_ELEMENT succeeds, so we don't free the disk we've just added to the disk list. VIR_APPEND_ELEMENT(, , disk) sets disk=NULL on success, precisely so you don't have to worry about double-freeing it (if you call the macro, you are guaranteed either transfer semantics and success, or no change on failure). Ah, should have read the source :). Thanks Eric. That explains my confusion as to why a crash was never reported in this code (or the vif parsing code). Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list