[libvirt] [PATCH] qemu: Allow system pages to <memoryBacking/>
by Michal Privoznik
It occurred to me that OpenStack uses the following XML when not using
regular huge pages:
<memoryBacking>
<hugepages>
<page size='4' unit='KiB'/>
</hugepages>
</memoryBacking>
However, since we are expecting to see huge pages only, we fail to
startup the domain with following error:
libvirtError: internal error: Unable to find any usable hugetlbfs
mount for 4 KiB
While regular system pages are not huge pages technically, our code is
prepared for that and if it helps OpenStack (or other management
applications) we should cope with that.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 84 +++++++++++++---------
.../qemuxml2argv-hugepages-pages6.args | 4 ++
.../qemuxml2argv-hugepages-pages6.xml | 32 +++++++++
tests/qemuxml2argvtest.c | 1 +
4 files changed, 87 insertions(+), 34 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 48bdf4e..c44560f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6616,6 +6616,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
char *nodemask = NULL;
char *mem_path = NULL;
int ret = -1;
+ const long system_page_size = sysconf(_SC_PAGESIZE);
if (virDomainNumatuneHasPerNodeBinding(def->numatune) &&
!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
@@ -6626,7 +6627,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
- if (def->mem.nhugepages && def->mem.hugepages[0].size &&
+ if (def->mem.nhugepages &&
+ def->mem.hugepages[0].size != system_page_size &&
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("huge pages per NUMA node are not "
@@ -6720,6 +6722,13 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
hugepage = master_hugepage;
}
+ if (hugepage && hugepage->size == system_page_size) {
+ /* However, if user specified to use "huge" page
+ * of regular system page size, it's as if they
+ * hasn't specified any huge pages at all. */
+ hugepage = NULL;
+ }
+
if (hugepage) {
/* Now lets see, if the huge page we want to use is even mounted
* and ready to use */
@@ -7882,44 +7891,51 @@ qemuBuildCommandLine(virConnectPtr conn,
def->mem.max_balloon = VIR_DIV_UP(def->mem.max_balloon, 1024) * 1024;
virCommandAddArgFormat(cmd, "%llu", def->mem.max_balloon / 1024);
if (def->mem.nhugepages && (!def->cpu || !def->cpu->ncells)) {
- char *mem_path;
+ const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+ char *mem_path = NULL;
- if (!cfg->nhugetlbfs) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("hugetlbfs filesystem is not mounted "
- "or disabled by administrator config"));
- goto error;
- }
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_PATH)) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("hugepage backing not supported by '%s'"),
- def->emulator);
- goto error;
- }
-
- if (def->mem.hugepages[0].size) {
- for (j = 0; j < cfg->nhugetlbfs; j++) {
- if (cfg->hugetlbfs[j].size == def->mem.hugepages[0].size)
- break;
- }
-
- if (j == cfg->nhugetlbfs) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to find any usable hugetlbfs mount for %llu KiB"),
- def->mem.hugepages[0].size);
- goto error;
- }
-
- if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[j])))
- goto error;
+ if (def->mem.hugepages[0].size == system_page_size) {
+ /* There is one special case: if user specified "huge"
+ * pages of regular system pages size. */
} else {
- if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs,
- cfg->nhugetlbfs)))
+ if (!cfg->nhugetlbfs) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("hugetlbfs filesystem is not mounted "
+ "or disabled by administrator config"));
goto error;
+ }
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_PATH)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("hugepage backing not supported by '%s'"),
+ def->emulator);
+ goto error;
+ }
+
+ if (def->mem.hugepages[0].size) {
+ for (j = 0; j < cfg->nhugetlbfs; j++) {
+ if (cfg->hugetlbfs[j].size == def->mem.hugepages[0].size)
+ break;
+ }
+
+ if (j == cfg->nhugetlbfs) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to find any usable hugetlbfs mount for %llu KiB"),
+ def->mem.hugepages[0].size);
+ goto error;
+ }
+
+ if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[j])))
+ goto error;
+ } else {
+ if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs,
+ cfg->nhugetlbfs)))
+ goto error;
+ }
}
- virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path",
- mem_path, NULL);
+ virCommandAddArg(cmd, "-mem-prealloc");
+ if (mem_path)
+ virCommandAddArgList(cmd, "-mem-path", mem_path, NULL);
VIR_FREE(mem_path);
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args
new file mode 100644
index 0000000..4eccb86
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args
@@ -0,0 +1,4 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 1024 -mem-prealloc -smp 2 -nographic \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
+-hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.xml
new file mode 100644
index 0000000..4f318fd
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>SomeDummyHugepagesGuest</name>
+ <uuid>ef1bdff4-27f3-4e85-a807-5fb4d58463cc</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <memoryBacking>
+ <hugepages>
+ <page size='4' unit='KiB'/>
+ </hugepages>
+ </memoryBacking>
+ <vcpu placement='static'>2</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</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>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 9e39068..bce88a8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -713,6 +713,7 @@ mymain(void)
DO_TEST_FAILURE("hugepages-pages4", QEMU_CAPS_MEM_PATH,
QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
DO_TEST("hugepages-pages5", QEMU_CAPS_MEM_PATH);
+ DO_TEST("hugepages-pages6", NONE);
DO_TEST("nosharepages", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MEM_MERGE);
DO_TEST("disk-cdrom", NONE);
DO_TEST("disk-cdrom-network-http", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE,
--
2.0.4
10 years, 3 months
[libvirt] [PATCH] conf: Ignore device address for model=none usb controller and memballon
by Luyao Huang
It make no sense at all to have it there.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/conf/domain_conf.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ec45b8c..2965d8d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6683,8 +6683,12 @@ virDomainControllerDefParseXML(xmlNodePtr node,
_("Malformed 'max_sectors' value %s'"), max_sectors);
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ def->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
+ VIR_DEBUG("Ignoring device address for none model usb controller");
+ } else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) {
goto error;
+ }
switch (def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: {
@@ -9989,7 +9993,9 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
goto error;
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (def->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)
+ VIR_DEBUG("Ignoring device address for none model Memballoon");
+ else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
goto error;
cleanup:
--
1.8.3.1
10 years, 3 months
[libvirt] [PATCH v4 00/17] LXC IP configuration feature
by Cédric Bosdonnat
Hi all,
Here is an updated version of the lxc IP configuration patch series.
Diffs to v3:
* Took care of Daniel's feedback
* Fixed openvz driver to add support for multiple --ipadd
* Report an error for drivers using IP addresses, but only the first one
in case there are more provided.
* Squashed Patch 11 to have virDomainNetIpsFormat right in the first place
* Fixed regression introduced in qemuConnectDomainXMLToNative: IPs were not
kept over memset for bridge type.
* Reformatted to fit latest make syntax-check changes
Cédric Bosdonnat (17):
Forgot to cleanup ifname_guest* in domain network def parsing
Domain conf: allow more than one IP address for net devices
LXC: set IP addresses to veth devices in the container
lxc conf2xml: convert IP addresses
Allow network capabilities hostdev to configure IP addresses
lxc conf2xml: convert ip addresses for hostdev NICs
Domain network devices can now have a <gateway> element
lxc conf2xml: convert lxc.network.ipv[46].gateway
LXC: use the new net devices gateway definition
LXC: honour network devices link state
virNetDevSetIPv4Address: libnl implementation
Renamed virNetDevSetIPv4Address to virNetDevSetIPAddress
virNetDevAddRoute: implementation using netlink
virNetDevClearIPv4Address: netlink implementation
Renamed virNetDevClearIPv4Address to virNetDevClearIPAddress
Openvz --ipadd can be provided multiple times
Report error if a driver can't handle multiple IP addresses
docs/formatdomain.html.in | 39 +++
docs/schemas/domaincommon.rng | 65 ++++-
src/conf/domain_conf.c | 251 +++++++++++++++--
src/conf/domain_conf.h | 21 +-
src/libvirt_private.syms | 6 +-
src/lxc/lxc_container.c | 69 ++++-
src/lxc/lxc_native.c | 165 +++++++----
src/network/bridge_driver.c | 4 +-
src/openvz/openvz_conf.c | 2 +-
src/openvz/openvz_driver.c | 11 +-
src/qemu/qemu_driver.c | 29 +-
src/qemu/qemu_hotplug.c | 5 +-
src/uml/uml_conf.c | 2 +-
src/util/virnetdev.c | 305 ++++++++++++++++++---
src/util/virnetdev.h | 12 +-
src/util/virnetlink.c | 38 +++
src/util/virnetlink.h | 2 +
src/util/virsocketaddr.h | 3 +
src/vbox/vbox_common.c | 16 +-
src/xenconfig/xen_common.c | 29 +-
src/xenconfig/xen_sxpr.c | 26 +-
.../lxcconf2xmldata/lxcconf2xml-physnetwork.config | 4 +
tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml | 4 +
tests/lxcconf2xmldata/lxcconf2xml-simple.config | 4 +
tests/lxcconf2xmldata/lxcconf2xml-simple.xml | 4 +
tests/lxcxml2xmldata/lxc-hostdev.xml | 4 +
tests/lxcxml2xmldata/lxc-idmap.xml | 3 +
tests/openvzutilstest.c | 2 +-
tests/sexpr2xmldata/sexpr2xml-bridge-ipaddr.xml | 2 +-
tests/sexpr2xmldata/sexpr2xml-net-routed.xml | 2 +-
30 files changed, 963 insertions(+), 166 deletions(-)
--
2.1.2
10 years, 3 months
[libvirt] [PATCH] CVE-2014-8131: Fix possible deadlock and segfault in qemuConnectGetAllDomainStats()
by Martin Kletzander
When user doesn't have read access on one of the domains he requested,
the for loop could exit abruptly or continue and override pointer which
pointed to locked object.
This patch fixed two issues at once. One is that domflags might have
had QEMU_DOMAIN_STATS_HAVE_JOB even when there was no job started (this
is fixed by doing domflags |= QEMU_DOMAIN_STATS_HAVE_JOB only when the
job was acquired and cleaning domflags on every start of the loop.
Second one is that the domain is kept locked when
virConnectGetAllDomainStatsCheckACL() fails and continues the loop when
it didn't end. Adding a simple virObjectUnlock() and clearing the
pointer ought to do.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
Since this is very low priority and it was mistakenly disclosed in
another patch there is no embargo on this CVE. Patches to maint
branches will be pushed after back-porting (couple of minutes).
Having said that, I am probably not the right one to write up the
libvirt security notice, but I'll try drafting one, eventually.
src/qemu/qemu_driver.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index be37c8f..ae6225b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18740,20 +18740,23 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
privflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
for (i = 0; i < ndoms; i++) {
- domflags = privflags;
virDomainStatsRecordPtr tmp = NULL;
+ domflags = 0;
if (!(dom = qemuDomObjFromDomain(doms[i])))
continue;
if (doms != domlist &&
- !virConnectGetAllDomainStatsCheckACL(conn, dom->def))
+ !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) {
+ virObjectUnlock(dom);
+ dom = NULL;
continue;
+ }
- if (HAVE_JOB(domflags) &&
+ if (HAVE_JOB(privflags) &&
qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0)
/* As it was never requested. Gather as much as possible anyway. */
- domflags &= ~QEMU_DOMAIN_STATS_HAVE_JOB;
+ domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0)
goto endjob;
@@ -18761,9 +18764,12 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
if (tmp)
tmpstats[nstats++] = tmp;
- if (HAVE_JOB(domflags) && !qemuDomainObjEndJob(driver, dom)) {
- dom = NULL;
- continue;
+ if (HAVE_JOB(domflags)) {
+ domflags = 0;
+ if (!qemuDomainObjEndJob(driver, dom)) {
+ dom = NULL;
+ continue;
+ }
}
virObjectUnlock(dom);
--
2.2.0
10 years, 3 months
[libvirt] [PATCH] qemu: bulk stats: typo in monitor handling
by Francesco Romani
A typo in qemuConnectGetAllDomainStats makes the code
mark the monitor as available when qemuDomainObjBeginJob
fails, instead of when it succeeds, as the correct flow
requires.
This patch fixes the check and updates the code documentation
accordingly.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
src/qemu/qemu_driver.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 830fca7..129e10c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18745,9 +18745,12 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
}
if (HAVE_JOB(privflags) &&
- qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0)
- /* As it was never requested. Gather as much as possible anyway. */
+ qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) == 0)
domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
+ /*
+ * else: as it was never requested.
+ * Gather as much as possible anyway.
+ */
if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0)
goto endjob;
--
1.9.3
10 years, 3 months
[libvirt] [PATCH] qemu: bulk stats: Clean up code to check whether we need a domain job
by Peter Krempa
Storing the information as a flag in a unsigned int seems a bit
wasteful. Store it in a bool instead.
---
src/qemu/qemu_driver.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index df3ba6d..5c60127 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18691,7 +18691,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
int nstats = 0;
size_t i;
int ret = -1;
- unsigned int privflags = 0;
+ bool needjob;
unsigned int domflags = 0;
if (ndoms)
@@ -18727,8 +18727,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
if (VIR_ALLOC_N(tmpstats, ndoms + 1) < 0)
goto cleanup;
- if (qemuDomainGetStatsNeedMonitor(stats))
- privflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
+ bool needjob = qemuDomainGetStatsNeedMonitor(stats);
for (i = 0; i < ndoms; i++) {
virDomainStatsRecordPtr tmp = NULL;
@@ -18744,7 +18743,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
continue;
}
- if (HAVE_JOB(privflags) &&
+ if (needjob &&
qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) == 0)
domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
/* else: without a job it's still possible to gather some data */
--
2.2.0
10 years, 3 months
Re: [libvirt] [virt-tools-list] [PATCH] virtinst: refresh pools status before fetch_pools
by Chun Yan Liu
>>> On 12/5/2014 at 09:54 PM, in message <5481B907.4040507(a)redhat.com>, Cole
Robinson <crobinso(a)redhat.com> wrote:
> On 12/05/2014 03:40 AM, Chunyan Liu wrote:
> > Currently, when connecting to hypervisor, if there are pools active
> > but in fact target path already deleted (or for other reasons the
> > pool is not working), libvirtd not refresh status yet, fetch_pools
> > will fail, that will cause "connecting to hypervisor" process
> > reporting error and exit. The whole connection work failed.
> >
> > With the patch, always refresh pool status before fetch pools. Let
> > the libvirtd pool status reflect the reality, avoid the non-synced
> > status affects the hypervisor connection.
> >
> > Signed-off-by: Chunyan Liu <cyliu(a)suse.com>
> > ---
> > virtinst/pollhelpers.py | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/virtinst/pollhelpers.py b/virtinst/pollhelpers.py
> > index a9b1527..e8702f0 100644
> > --- a/virtinst/pollhelpers.py
> > +++ b/virtinst/pollhelpers.py
> > @@ -133,6 +133,19 @@ def fetch_pools(backend, origmap, build_func):
> >
> > if backend.check_support(
> > backend.SUPPORT_CONN_LISTALLSTORAGEPOOLS) and not
> _force_old_poll:
> > +
> > + # Refresh pools before poll_helper. For those
> > + # 'active' but target path not exist (or other reasons
> > + # causing the pool not working), but libvirtd not
> > + # refresh the status, this will make it refreshed
> > + # and mark that pool as 'inactive'.
> > + objs = backend.listAllStoragePools()
> > + for obj in objs:
> > + try:
> > + obj.refresh(0)
> > + except Exception, e:
> > + pass
> > +
> > return _new_poll_helper(origmap, name,
> > backend.listAllStoragePools, build_func)
> > else:
> >
>
> This is a very heavy hammer, refresh is a potentially long running operation
>
> so this could cause decent slowdown in some scenarios.
>
> IMO this is essentially a libvirt bug, for pools with target directories
> (dir,
> fs, netfs), libvirt should be periodically checking the directory ctime and
> doing the pool refresh for us. And if the target has disappeared, it shuts
> down the pool (like shutting down a VM if it crashes).
Hi, libvirt list,
I'm not sure if Cole's suggestion could be done in libvirt, so just forward
the mail to libvirt mailing list. Any opinions?
Chunyan
>
> We have so many hacks sprinkled around in virtinst/virt-manager dealing with
> the fallout of this lacking libvirt feature. Really wish I had implemented
> it
> years ago. But I'd rather focus on that then adding yet more hacks.
>
> If there's a specific issue you're hitting that's manifesting itself
> elsewhere
> in the app, let us know and maybe there's a way to mitigate it in the
> interim
>
> - Cole
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
>
>
10 years, 3 months
[libvirt] [PATCH] storage: unify permission formatting
by Martin Kletzander
Volume and pool formatting functions took different approaches to
unspecified uids/gids. When unknown, it is always parsed as -1, but one
of the functions formatted it as unsigned int (wrong) and one as
int (better). Due to that, our two of our XML files from tests cannot
be parsed on 32-bit machines.
RNG schema needs to be modified as well, but because both
storagepool.rng and storagevol.rng need same schema for permission
element, save some space by moving it to storagecommon.rng.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
docs/schemas/storagecommon.rng | 29 +++++++++++++++++++++++++
docs/schemas/storagepool.rng | 30 +-------------------------
docs/schemas/storagevol.rng | 23 --------------------
src/conf/storage_conf.c | 9 ++++----
tests/storagevolxml2xmlout/vol-gluster-dir.xml | 4 ++--
tests/storagevolxml2xmlout/vol-sheepdog.xml | 4 ++--
6 files changed, 38 insertions(+), 61 deletions(-)
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 06b2f81..629505f 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -93,4 +93,33 @@
<notAllowed/>
</define>
+ <define name='permissions'>
+ <optional>
+ <element name='permissions'>
+ <interleave>
+ <element name='mode'>
+ <ref name='octalMode'/>
+ </element>
+ <element name='owner'>
+ <choice>
+ <ref name='unsignedInt'/>
+ <value>-1</value>
+ </choice>
+ </element>
+ <element name='group'>
+ <choice>
+ <ref name='unsignedInt'/>
+ <value>-1</value>
+ </choice>
+ </element>
+ <optional>
+ <element name='label'>
+ <text/>
+ </element>
+ </optional>
+ </interleave>
+ </element>
+ </optional>
+ </define>
+
</grammar>
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 0f05c5c..db6ff49 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -3,6 +3,7 @@
<grammar xmlns="http://relaxng.org/ns/structure/1.0"
datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes">
<include href='basictypes.rng'/>
+ <include href='storagecommon.rng'/>
<start>
<ref name='pool'/>
</start>
@@ -224,35 +225,6 @@
</interleave>
</define>
- <define name='permissions'>
- <optional>
- <element name='permissions'>
- <interleave>
- <element name='mode'>
- <ref name='octalMode'/>
- </element>
- <element name='owner'>
- <choice>
- <ref name='unsignedInt'/>
- <value>-1</value>
- </choice>
- </element>
- <element name='group'>
- <choice>
- <ref name='unsignedInt'/>
- <value>-1</value>
- </choice>
- </element>
- <optional>
- <element name='label'>
- <text/>
- </element>
- </optional>
- </interleave>
- </element>
- </optional>
- </define>
-
<define name='target'>
<element name='target'>
<interleave>
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index 1b2d4cc..7450547 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -59,29 +59,6 @@
</interleave>
</define>
- <define name='permissions'>
- <optional>
- <element name='permissions'>
- <interleave>
- <element name='mode'>
- <ref name='octalMode'/>
- </element>
- <element name='owner'>
- <ref name='unsignedInt'/>
- </element>
- <element name='group'>
- <ref name='unsignedInt'/>
- </element>
- <optional>
- <element name='label'>
- <text/>
- </element>
- </optional>
- </interleave>
- </element>
- </optional>
- </define>
-
<define name='timestamps'>
<optional>
<element name='timestamps'>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 3987470..e1be064 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1203,7 +1203,6 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def)
(int) def->target.perms.uid);
virBufferAsprintf(&buf, "<group>%d</group>\n",
(int) def->target.perms.gid);
-
virBufferEscapeString(&buf, "<label>%s</label>\n",
def->target.perms.label);
@@ -1527,10 +1526,10 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
virBufferAsprintf(buf, "<mode>0%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, "<owner>%d</owner>\n",
+ (int) def->perms->uid);
+ virBufferAsprintf(buf, "<group>%d</group>\n",
+ (int) def->perms->gid);
virBufferEscapeString(buf, "<label>%s</label>\n",
diff --git a/tests/storagevolxml2xmlout/vol-gluster-dir.xml b/tests/storagevolxml2xmlout/vol-gluster-dir.xml
index f188ceb..538b31d 100644
--- a/tests/storagevolxml2xmlout/vol-gluster-dir.xml
+++ b/tests/storagevolxml2xmlout/vol-gluster-dir.xml
@@ -10,8 +10,8 @@
<format type='dir'/>
<permissions>
<mode>0600</mode>
- <owner>4294967295</owner>
- <group>4294967295</group>
+ <owner>-1</owner>
+ <group>-1</group>
</permissions>
</target>
</volume>
diff --git a/tests/storagevolxml2xmlout/vol-sheepdog.xml b/tests/storagevolxml2xmlout/vol-sheepdog.xml
index e08e36c..0a1f32c 100644
--- a/tests/storagevolxml2xmlout/vol-sheepdog.xml
+++ b/tests/storagevolxml2xmlout/vol-sheepdog.xml
@@ -9,8 +9,8 @@
<format type='unknown'/>
<permissions>
<mode>0600</mode>
- <owner>4294967295</owner>
- <group>4294967295</group>
+ <owner>-1</owner>
+ <group>-1</group>
</permissions>
</target>
</volume>
--
2.2.0
10 years, 3 months
[libvirt] [PATCHv4 2/2] security: Add a new func use stat to get process DAC label
by Luyao Huang
When use qemuProcessAttach to attach a qemu process, cannot
get a right DAC label. Add a new func to get process label
via stat func. Do not remove virDomainDefGetSecurityLabelDef
before try to use stat to get process DAC label, because
There are some other func call virSecurityDACGetProcessLabel.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
v2 add support freeBSD.
v3 use snprintf instead of VirAsprintf and move the error
settings in virSecurityDACGetProcessLabelInternal.
v4 remove errno.h include and thanks Eric advice move this
version comment to this place.
src/security/security_dac.c | 84 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 81 insertions(+), 3 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 85253af..300b245 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -23,6 +23,11 @@
#include <sys/stat.h>
#include <fcntl.h>
+#ifdef __FreeBSD__
+# include <sys/sysctl.h>
+# include <sys/user.h>
+#endif
+
#include "security_dac.h"
#include "virerror.h"
#include "virfile.h"
@@ -1236,17 +1241,90 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
return 0;
}
+#ifdef __linux__
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+ struct stat sb;
+ char *path = NULL;
+ int ret = -1;
+
+ VIR_INFO("Getting DAC user and group on process '%d'", pid);
+
+ if (virAsprintf(&path, "/proc/%d", (int) pid) < 0)
+ goto cleanup;
+
+ if (lstat(path, &sb) < 0) {
+ virReportSystemError(errno,
+ _("unable to get PID %d uid and gid via stat"),
+ pid);
+ goto cleanup;
+ }
+
+ snprintf(seclabel->label, VIR_SECURITY_LABEL_BUFLEN,
+ "+%u:+%u", (unsigned int) sb.st_uid, (unsigned int) sb.st_gid);
+ ret = 0;
+
+cleanup:
+ VIR_FREE(path);
+ return ret;
+}
+#elif defined(__FreeBSD__)
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+ struct kinfo_proc p;
+ int mib[4];
+ size_t len = 4;
+
+ sysctlnametomib("kern.proc.pid", mib, &len);
+
+ len = sizeof(struct kinfo_proc);
+ mib[3] = pid;
+
+ if (sysctl(mib, 4, &p, &len, NULL, 0) < 0) {
+ virReportSystemError(errno,
+ _("unable to get PID %d uid and gid via sysctl"),
+ pid);
+ return -1;
+ }
+
+ snprintf(seclabel->label, VIR_SECURITY_LABEL_BUFLEN,
+ "+%u:+%u", (unsigned int) p.ki_ruid, (unsigned int) p.ki_rgid);
+
+ return 0;
+}
+#else
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ "Cannot get proccess DAC label for pid %d on this platform",
+ (int) pid);
+ return -1;
+}
+#endif
+
static int
virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr def,
- pid_t pid ATTRIBUTE_UNUSED,
+ pid_t pid,
virSecurityLabelPtr seclabel)
{
virSecurityLabelDefPtr secdef =
virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
- if (!secdef || !seclabel)
- return -1;
+ if (secdef == NULL) {
+ VIR_DEBUG("missing label for DAC security "
+ "driver in domain %s", def->name);
+
+ if (virSecurityDACGetProcessLabelInternal(pid, seclabel) < 0)
+ return -1;
+ return 0;
+ }
if (secdef->label)
ignore_value(virStrcpy(seclabel->label, secdef->label,
--
1.8.3.1
10 years, 3 months
