Re: [libvirt] [PATCH] Fix parallel runs of TLS test suites
On 08/13/2013 04:20 AM, Eric Blake wrote: On 08/09/2013 03:23 AM, Daniel P. Berrange wrote: Anyway, why I'm saying this is that one more filename should be renamed in order to avoid a race (which I was unable to reproduce, though). I have reproduced the race. ACK, yes this is also needed ontop of my patch. I went ahead and pushed this in your name, since it didn't make it into Dan's original patch, and since I was hitting it. Thank you, I wasn't checking whether this got squashed in or not. I'm glad to hear the patch fixed your issue. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] Add pcihole64 attribute to root PCI controllers
controller type='pci' index='0' model='pci-root' pcihole64='1'/
It can be used to adjust (or disable) the size of the 64-bit
PCI hole. The size attribute is in gigabytes, since it would
get rounded up to nearest GB by QEMU anyway.
Disabling it will be needed for guests that crash with the
64-bit PCI hole (like Windows XP), see:
https://bugzilla.redhat.com/show_bug.cgi?id=990418
---
docs/formatdomain.html.in | 8 ++
docs/schemas/domaincommon.rng | 32 --
src/conf/domain_conf.c | 17
src/conf/domain_conf.h | 8 ++
.../qemuxml2argv-pcihole64-none.xml| 21 ++
.../qemuxml2argv-pcihole64-q35.xml | 31 +
tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml | 21 ++
tests/qemuxml2xmltest.c| 4 +++
8 files changed, 134 insertions(+), 8 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 83d551a..3475022 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2344,6 +2344,14 @@
PCI controllers have an optional codemodel/code attribute with
possible values codepci-root/code, codepcie-root/code,
codepci-bridge/code, or codedmi-to-pci-bridge/code.
+ The root controllers (codepci-root/code and codepcie-root/code)
+ have an optional codepcihole64/code attribute specifying how big
+ (in gigabytes) the 64-bit PCI hole should be. Some guests (like
+ Windows XP or Windows Server 2003) might crash when QEMU and Seabios
+ are recent enough to support 64-bit PCI holes, unless this is disabled
+ (set to 0). span class=sinceSince 1.1.2 (QEMU only)/span
+/p
+p
For machine types which provide an implicit PCI bus, the pci-root
controller with index=0 is auto-added and required to use PCI devices.
pci-root has no address.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ac807e6..2cff4d8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1541,14 +1541,30 @@
attribute name=type
valuepci/value
/attribute
-attribute name=model
- choice
-valuepci-root/value
-valuepcie-root/value
-valuepci-bridge/value
-valuedmi-to-pci-bridge/value
- /choice
-/attribute
+!-- *-root controllers have an optional attribute pcihole64--
+choice
+ group
+attribute name=model
+ choice
+valuepci-root/value
+valuepcie-root/value
+ /choice
+/attribute
+optional
+ attribute name=pcihole64
+ref name=unsignedInt/
+ /attribute
+/optional
+ /group
+ group
+attribute name=model
+ choice
+valuepci-bridge/value
+valuedmi-to-pci-bridge/value
+ /choice
+/attribute
+ /group
+/choice
/group
!-- virtio-serial has optional ports and vectors --
group
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7309877..5c044e9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5614,6 +5614,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
char *idx = NULL;
char *model = NULL;
char *queues = NULL;
+char *pcihole64 = NULL;
if (VIR_ALLOC(def) 0)
return NULL;
@@ -5737,6 +5738,16 @@ virDomainControllerDefParseXML(xmlNodePtr node,
should have index 0));
goto error;
}
+if ((pcihole64 = virXMLPropString(node, pcihole64))) {
+def-opts.pciopts.pcihole64 = true;
+if (virStrToLong_ui(pcihole64, NULL, 10,
+def-opts.pciopts.pcihole64size) 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Cannot parse pcihole64 size %s),
+ pcihole64);
+goto error;
+}
+}
}
@@ -14491,6 +14502,12 @@ virDomainControllerDefFormat(virBufferPtr buf,
}
break;
+case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+if (def-opts.pciopts.pcihole64)
+virBufferAsprintf(buf, pcihole64='%u',
+
[libvirt] [PATCH v2 2/2] Build QEMU command line for pcihole64
QEMU commit 3984890 introduced the pci-hole64-size property,
to i440FX-pcihost and q35-pcihost with a default setting of 2 GB.
Translate controller ... pcihole64='x'/ to:
-global q35-pcihost.pci-hole64-size=x for q35 machines and
-global i440FX-pcihost.pci-hole64-size=x for i440FX-based machines.
Error out on other machine types or if the size was specified
but the pcihost device lacks 'pci-hole64-size' property.
https://bugzilla.redhat.com/show_bug.cgi?id=990418
---
src/qemu/qemu_capabilities.c | 14 ++
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_command.c| 58 ++
.../qemuxml2argv-pcihole64-none.args | 4 ++
.../qemuxml2argv-pcihole64-q35.args| 9
tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args | 5 ++
tests/qemuxml2argvtest.c | 10
7 files changed, 102 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 47cc07a..87c9a96 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -235,6 +235,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
vnc-share-policy, /* 150 */
device-del-event,
dmi-to-pci-bridge,
+ i440fx-pci-hole64-size,
+ q35-pci-hole64-size,
);
struct _virQEMUCaps {
@@ -1436,6 +1438,14 @@ static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsScsiGeneric[] = {
{ bootindex, QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX },
};
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsI440FXPciHost[] = {
+{ pci-hole64-size, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE },
+};
+
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQ35PciHost[] = {
+{ pci-hole64-size, QEMU_CAPS_Q35_PCI_HOLE64_SIZE },
+};
+
struct virQEMUCapsObjectTypeProps {
const char *type;
struct virQEMUCapsStringFlags *props;
@@ -1473,6 +1483,10 @@ static struct virQEMUCapsObjectTypeProps
virQEMUCapsObjectProps[] = {
ARRAY_CARDINALITY(virQEMUCapsObjectPropsUsbHost) },
{ scsi-generic, virQEMUCapsObjectPropsScsiGeneric,
ARRAY_CARDINALITY(virQEMUCapsObjectPropsScsiGeneric) },
+{ i440FX-pcihost, virQEMUCapsObjectPropsI440FXPciHost,
+ ARRAY_CARDINALITY(virQEMUCapsObjectPropsI440FXPciHost) },
+{ q35-pcihost, virQEMUCapsObjectPropsQ35PciHost,
+ ARRAY_CARDINALITY(virQEMUCapsObjectPropsQ35PciHost) },
};
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 074e55d..69f3395 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -191,6 +191,8 @@ enum virQEMUCapsFlags {
QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */
QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */
+QEMU_CAPS_I440FX_PCI_HOLE64_SIZE = 153, /* i440FX-pcihost.pci-hole64-size
*/
+QEMU_CAPS_Q35_PCI_HOLE64_SIZE = 154, /* q35-pcihost.pci-hole64-size */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b811e1d..3202385 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2399,6 +2399,17 @@ qemuDomainMachineIsQ35(virDomainDefPtr def)
}
+static bool
+qemuDomainMachineIsI440FX(virDomainDefPtr def)
+{
+return (STREQ(def-os.machine, pc) ||
+STRPREFIX(def-os.machine, pc-0.) ||
+STRPREFIX(def-os.machine, pc-1.) ||
+STRPREFIX(def-os.machine, pc-i440) ||
+STRPREFIX(def-os.machine, rhel));
+}
+
+
static int
qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
@@ -7919,6 +7930,53 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArgList(cmd, -bootloader, def-os.bootloader, NULL);
}
+for (i = 0; i def-ncontrollers; i++) {
+virDomainControllerDefPtr cont = def-controllers[i];
+if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI
+cont-opts.pciopts.pcihole64) {
+const char *hoststr = NULL;
+bool cap = false;
+bool machine = false;
+
+switch (cont-model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+hoststr = i440FX-pcihost;
+cap = virQEMUCapsGet(qemuCaps,
QEMU_CAPS_I440FX_PCI_HOLE64_SIZE);
+machine = qemuDomainMachineIsI440FX(def);
+break;
+
+case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+hoststr = q35-pcihost;
+cap = virQEMUCapsGet(qemuCaps,
[libvirt] [PATCH v2 0/2] Add support for adjusting the 64-bit PCI hole size
v2: Use 'pcihole64' attribute of the root PCI controller instead of pcihole64 element in domain features. v1: https://www.redhat.com/archives/libvir-list/2013-August/msg00510.html https://bugzilla.redhat.com/show_bug.cgi?id=990418 Ján Tomko (2): Add pcihole64 attribute to root PCI controllers Build QEMU command line for pcihole64 docs/formatdomain.html.in | 8 +++ docs/schemas/domaincommon.rng | 32 +--- src/conf/domain_conf.c | 17 +++ src/conf/domain_conf.h | 8 +++ src/qemu/qemu_capabilities.c | 14 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c| 58 ++ .../qemuxml2argv-pcihole64-none.args | 4 ++ .../qemuxml2argv-pcihole64-none.xml| 21 .../qemuxml2argv-pcihole64-q35.args| 9 .../qemuxml2argv-pcihole64-q35.xml | 31 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args | 5 ++ tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml | 21 tests/qemuxml2argvtest.c | 10 tests/qemuxml2xmltest.c| 4 ++ 15 files changed, 236 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Link to virdbustest against DBus libs
On Mon, Aug 12, 2013 at 10:13:07PM +0200, Guido Günther wrote: On Mon, Aug 12, 2013 at 01:21:08PM -0600, Eric Blake wrote: On 08/12/2013 01:14 PM, Guido Günther wrote: otherwise we fail like: CCLD virdbustest /usr/bin/ld: virdbustest-virdbustest.o: undefined reference to symbol 'dbus_message_unref' /lib/x86_64-linux-gnu/libdbus-1.so.3: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status Found by: http://honk.sigxcpu.org:8001/job/libvirt-build-debian-sid-amd64/7/console --- tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. I found several more places which fail to link with a strict --no-copy-dt-needed-entries. See my followup patch which superseeds this one. Hmm, it sounds like we ought to make configure.ac automatically add this linker flag if we can detect that it is supported. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] Add pcihole64 attribute to root PCI controllers
On Tue, Aug 13, 2013 at 10:51:04AM +0200, Ján Tomko wrote: controller type='pci' index='0' model='pci-root' pcihole64='1'/ It can be used to adjust (or disable) the size of the 64-bit PCI hole. The size attribute is in gigabytes, since it would get rounded up to nearest GB by QEMU anyway. Choosing the units based on what one specific hypervisor happens to currently do has proven to be a pretty bad idea in the past. I'd say we should be using KB here. Or better yet, have this as a separate child element, and then support a 'units' attribute at the same time, defaulting to KB. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh-domain: Flip logic in cmdSetvcpus
To avoid having to assign a failure code to the returned variable switch
this function to negative logic. This will fix issue with invalid number
of cpus returning success return code.
https://bugzilla.redhat.com/show_bug.cgi?id=996466
---
tools/virsh-domain.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4081451..3b2513b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5965,7 +5965,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
int count = 0;
-bool ret = true;
+bool ret = false;
bool maximum = vshCommandOptBool(cmd, maximum);
bool config = vshCommandOptBool(cmd, config);
bool live = vshCommandOptBool(cmd, live);
@@ -5996,9 +5996,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
}
if (flags == -1) {
-if (virDomainSetVcpus(dom, count) != 0) {
-ret = false;
-}
+if (virDomainSetVcpus(dom, count) != 0)
+goto cleanup;
} else {
/* If the --maximum flag was given, we need to ensure only the
--config flag is in effect as well */
@@ -6015,18 +6014,18 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
/* Warn the user about the invalid flag combination */
vshError(ctl, _(--maximum must be used with --config only));
-ret = false;
goto cleanup;
}
}
/* Apply the virtual cpu changes */
-if (virDomainSetVcpusFlags(dom, count, flags) 0) {
-ret = false;
-}
+if (virDomainSetVcpusFlags(dom, count, flags) 0)
+goto cleanup;
}
- cleanup:
+ret = true;
+
+cleanup:
virDomainFree(dom);
return ret;
}
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh-domain: Check if domain is running for ttyconsole cmd
Signed-off-by: Yanbing Du y...@redhat.com
---
tools/virsh-domain.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4081451..9ccbc35 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9139,6 +9139,12 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
+/* Check if the domain is active */
+if (!virDomainIsActive(dom)) {
+vshError(ctl, _(Domain is not running));
+goto cleanup;
+}
+
doc = virDomainGetXMLDesc(dom, 0);
if (!doc)
goto cleanup;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] Add pcihole64 attribute to root PCI controllers
On 08/13/2013 10:55 AM, Daniel P. Berrange wrote: On Tue, Aug 13, 2013 at 10:51:04AM +0200, Ján Tomko wrote: controller type='pci' index='0' model='pci-root' pcihole64='1'/ It can be used to adjust (or disable) the size of the 64-bit PCI hole. The size attribute is in gigabytes, since it would get rounded up to nearest GB by QEMU anyway. Choosing the units based on what one specific hypervisor happens to currently do has proven to be a pretty bad idea in the past. I'd say we should be using KB here. Or better yet, have this as a separate child element, and then support a 'units' attribute at the same time, defaulting to KB. Would it be okay to use the largest usable unit when formatting the XML to make it more human-friendly? Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] Add pcihole64 attribute to root PCI controllers
On Tue, Aug 13, 2013 at 11:53:42AM +0200, Ján Tomko wrote: On 08/13/2013 10:55 AM, Daniel P. Berrange wrote: On Tue, Aug 13, 2013 at 10:51:04AM +0200, Ján Tomko wrote: controller type='pci' index='0' model='pci-root' pcihole64='1'/ It can be used to adjust (or disable) the size of the 64-bit PCI hole. The size attribute is in gigabytes, since it would get rounded up to nearest GB by QEMU anyway. Choosing the units based on what one specific hypervisor happens to currently do has proven to be a pretty bad idea in the past. I'd say we should be using KB here. Or better yet, have this as a separate child element, and then support a 'units' attribute at the same time, defaulting to KB. Would it be okay to use the largest usable unit when formatting the XML to make it more human-friendly? No, because you'd be throwing away data if the user had requested less than a GB. Outputting XML should use the smallest unit for which we want to support, which IMHO should be KB. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh-domain: Flip logic in cmdSetvcpus
On 08/13/2013 11:22 AM, Peter Krempa wrote: To avoid having to assign a failure code to the returned variable switch this function to negative logic. This will fix issue with invalid number of cpus returning success return code. https://bugzilla.redhat.com/show_bug.cgi?id=996466 --- tools/virsh-domain.c | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) ACK, nice reduction. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] Add pcihole64 attribute to root PCI controllers
On 08/13/2013 11:54 AM, Daniel P. Berrange wrote: On Tue, Aug 13, 2013 at 11:53:42AM +0200, Ján Tomko wrote: On 08/13/2013 10:55 AM, Daniel P. Berrange wrote: On Tue, Aug 13, 2013 at 10:51:04AM +0200, Ján Tomko wrote: controller type='pci' index='0' model='pci-root' pcihole64='1'/ It can be used to adjust (or disable) the size of the 64-bit PCI hole. The size attribute is in gigabytes, since it would get rounded up to nearest GB by QEMU anyway. Choosing the units based on what one specific hypervisor happens to currently do has proven to be a pretty bad idea in the past. I'd say we should be using KB here. Or better yet, have this as a separate child element, and then support a 'units' attribute at the same time, defaulting to KB. Would it be okay to use the largest usable unit when formatting the XML to make it more human-friendly? No, because you'd be throwing away data if the user had requested less than a GB. Outputting XML should use the smallest unit for which we want to support, which IMHO should be KB. By 'usable' I meant the unit that is the largest divisor of the size, e.g.: 512 KB would stay 512 KB, but 2048 MB would get translated to 2 GB. But this requires the applications parsing the XML to read both the size and the units. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] Add pcihole64 attribute to root PCI controllers
On Tue, Aug 13, 2013 at 12:24:51PM +0200, Ján Tomko wrote: On 08/13/2013 11:54 AM, Daniel P. Berrange wrote: On Tue, Aug 13, 2013 at 11:53:42AM +0200, Ján Tomko wrote: On 08/13/2013 10:55 AM, Daniel P. Berrange wrote: On Tue, Aug 13, 2013 at 10:51:04AM +0200, Ján Tomko wrote: controller type='pci' index='0' model='pci-root' pcihole64='1'/ It can be used to adjust (or disable) the size of the 64-bit PCI hole. The size attribute is in gigabytes, since it would get rounded up to nearest GB by QEMU anyway. Choosing the units based on what one specific hypervisor happens to currently do has proven to be a pretty bad idea in the past. I'd say we should be using KB here. Or better yet, have this as a separate child element, and then support a 'units' attribute at the same time, defaulting to KB. Would it be okay to use the largest usable unit when formatting the XML to make it more human-friendly? No, because you'd be throwing away data if the user had requested less than a GB. Outputting XML should use the smallest unit for which we want to support, which IMHO should be KB. By 'usable' I meant the unit that is the largest divisor of the size, e.g.: 512 KB would stay 512 KB, but 2048 MB would get translated to 2 GB. Ewww, no. The output unit should always be the same for any given attribute. But this requires the applications parsing the XML to read both the size and the units. Yep, this exactly why this is a bad idea Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Address missed feedback from review of virt-login-shell
From: Daniel P. Berrange berra...@redhat.com
Address a number of code, style and docs issues identified
in review of virt-login-shell after it was merged.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tools/Makefile.am | 1 -
tools/virt-login-shell.c | 58 ++
tools/virt-login-shell.pod | 30 ++--
3 files changed, 61 insertions(+), 28 deletions(-)
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 00c582a..d48883c 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -134,7 +134,6 @@ virt_host_validate_CFLAGS = \
$(NULL)
virt_login_shell_SOURCES = \
- virt-login-shell.conf \
virt-login-shell.c
virt_login_shell_LDFLAGS = $(COVERAGE_LDFLAGS)
diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index b27e44f..1157cd0 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -41,11 +41,11 @@
#include vircommand.h
#define VIR_FROM_THIS VIR_FROM_NONE
-static ssize_t nfdlist = 0;
-static int *fdlist = NULL;
+static ssize_t nfdlist;
+static int *fdlist;
static const char *conf_file = SYSCONFDIR /libvirt/virt-login-shell.conf;
-static void virLoginShellFini(virConnectPtr conn, virDomainPtr dom)
+static void virLoginShellFini(virConnectPtr conn, virDomainPtr dom)
{
size_t i;
@@ -105,7 +105,7 @@ static int virLoginShellAllowedUser(virConfPtr conf,
}
}
}
-virReportSystemError(EPERM, _(%s not listed as an allowed_users in %s),
name, conf_file);
+virReportSystemError(EPERM, _(%s not matched against 'allowed_users' in
%s), name, conf_file);
cleanup:
VIR_FREE(gname);
return ret;
@@ -121,7 +121,7 @@ static char **virLoginShellGetShellArgv(virConfPtr conf)
if (!p)
return virStringSplit(/bin/sh -l, , 3);
-if (p p-type == VIR_CONF_LIST) {
+if (p-type == VIR_CONF_LIST) {
size_t len;
virConfValuePtr pp;
@@ -139,7 +139,6 @@ static char **virLoginShellGetShellArgv(virConfPtr conf)
if (VIR_STRDUP(shargv[i], pp-str) 0)
goto error;
}
-shargv[len] = NULL;
}
return shargv;
error:
@@ -155,16 +154,27 @@ static char *progname;
static void
usage(void)
{
-fprintf(stdout, _(\n
- %s is a privileged program that allows non root users
\n
- specified in %s to join a Linux container \n
- with a matching user name and launch a shell. \n
- \n%s [options]\n\n
-options:\n
- -h | --help this help:\n\n), progname,
conf_file, progname);
+fprintf(stdout,
+_(\n
+ Usage:\n
+%s [options]\n\n
+ Options:\n
+-h | --helpDisplay program help:\n
+-V | --version Display program version:\n
+ \n
+ libvirt login shell\n),
+progname);
return;
}
+/* Display version information. */
+static void
+show_version(void)
+{
+printf(%s (%s) %s\n, progname, PACKAGE_NAME, PACKAGE_VERSION);
+}
+
+
int
main(int argc, char **argv)
{
@@ -190,6 +200,7 @@ main(int argc, char **argv)
struct option opt[] = {
{help, no_argument, NULL, 'h'},
+{version, optional_argument, NULL, 'V'},
{NULL, 0, NULL, 0}
};
if (virInitialize() 0) {
@@ -214,20 +225,25 @@ main(int argc, char **argv)
return ret;
}
-/* The only option we support is help
- */
-while ((arg = getopt_long(argc, argv, h, opt, longindex)) != -1) {
+while ((arg = getopt_long(argc, argv, hV, opt, longindex)) != -1) {
switch (arg) {
case 'h':
usage();
exit(EXIT_SUCCESS);
-break;
+
+case 'V':
+show_version();
+exit(EXIT_SUCCESS);
+
+case '?':
+default:
+usage();
+exit(EXIT_FAILURE);
}
}
if (argc optind) {
virReportSystemError(EINVAL, _(%s takes no options), progname);
-errno = EINVAL;
goto cleanup;
}
@@ -268,7 +284,9 @@ main(int argc, char **argv)
virErrorPtr last_error;
last_error = virGetLastError();
if (last_error-code != VIR_ERR_OPERATION_INVALID) {
-virReportSystemError(last_error-code,_(Can't create %s
container: %s), name, virGetLastErrorMessage());
+virReportSystemError(last_error-code,
+ _(Can't create %s container: %s),
+ name, last_error-message);
goto cleanup;
}
}
@@ -327,7 +345,7 @@ main(int argc, char **argv)
}
if (execv(shargv[0], (char *const*) shargv) 0) {
[libvirt] [PATCH] Properly handle -h / -V for --help/--version aliases in virtlockd/libvirtd
From: Daniel P. Berrange berra...@redhat.com
The virtlockd/libvirtd daemons had listed '?' as the short option
for --help. getopt_long uses '?' for any unknown option. We want
to be able to distinguish unknown options (which use EXIT_FAILURE)
from correct usage of help (which should use EXIT_SUCCESS). Thus
we should use 'h' as a short option for --help. Also add this to
the man page docs
The virtlockd/libvirtd daemons did not list any short option
for the --version arg. Add -V as a valid short option, since
-v is already used for --verbose.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
daemon/libvirtd.c| 31 ++-
daemon/libvirtd.pod.in | 4
src/locking/lock_daemon.c| 29 +
src/locking/virtlockd.pod.in | 6 +-
4 files changed, 36 insertions(+), 34 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 402b494..c9cd1a1 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1038,12 +1038,13 @@ daemonUsage(const char *argv0, bool privileged)
%s [options]\n
\n
Options:\n
+-h | --helpDisplay program help:\n
-v | --verbose Verbose messages.\n
-d | --daemon Run as a daemon write PID file.\n
-l | --listen Listen for TCP/IP connections.\n
-t | --timeout secs Exit after timeout period.\n
-f | --config file Configuration file.\n
- | --version Display version information.\n
+-V | --version Display version information.\n
-p | --pid-file file Change name of PID file.\n
\n
libvirt management daemon:\n),
@@ -1098,10 +1099,6 @@ daemonUsage(const char *argv0, bool privileged)
}
}
-enum {
-OPT_VERSION = 129
-};
-
#define MAX_LISTEN 5
int main(int argc, char **argv) {
virNetServerPtr srv = NULL;
@@ -1123,14 +1120,14 @@ int main(int argc, char **argv) {
mode_t old_umask;
struct option opts[] = {
-{ verbose, no_argument, verbose, 1},
-{ daemon, no_argument, godaemon, 1},
-{ listen, no_argument, ipsock, 1},
+{ verbose, no_argument, verbose, 'v'},
+{ daemon, no_argument, godaemon, 'd'},
+{ listen, no_argument, ipsock, 'l'},
{ config, required_argument, NULL, 'f'},
{ timeout, required_argument, NULL, 't'},
{ pid-file, required_argument, NULL, 'p'},
-{ version, no_argument, NULL, OPT_VERSION },
-{ help, no_argument, NULL, '?' },
+{ version, no_argument, NULL, 'V' },
+{ help, no_argument, NULL, 'h' },
{0, 0, 0, 0}
};
@@ -1173,7 +1170,7 @@ int main(int argc, char **argv) {
int c;
char *tmp;
-c = getopt_long(argc, argv, ldf:p:t:v, opts, optidx);
+c = getopt_long(argc, argv, ldf:p:t:vVh, opts, optidx);
if (c == -1) {
break;
@@ -1219,17 +1216,17 @@ int main(int argc, char **argv) {
}
break;
-case OPT_VERSION:
+case 'V':
daemonVersion(argv[0]);
-return 0;
+exit(EXIT_SUCCESS);
-case '?':
+case 'h':
daemonUsage(argv[0], privileged);
-return 2;
+exit(EXIT_SUCCESS);
+case '?':
default:
-VIR_ERROR(_(%s: internal error: unknown flag: %c),
- argv[0], c);
+daemonUsage(argv[0], privileged);
exit(EXIT_FAILURE);
}
}
diff --git a/daemon/libvirtd.pod.in b/daemon/libvirtd.pod.in
index 930b752..9901ecf 100644
--- a/daemon/libvirtd.pod.in
+++ b/daemon/libvirtd.pod.in
@@ -36,6 +36,10 @@ from the configuration.
=over
+=item B-h, --help
+
+Display command line help usage then exit.
+
=item B-d, --daemon
Run as a daemon write PID file.
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index c45f45c..77d6e0d 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -1096,10 +1096,11 @@ virLockDaemonUsage(const char *argv0, bool privileged)
%s [options]\n
\n
Options:\n
+-h | --helpDisplay program help:\n
-v | --verbose Verbose messages.\n
-d | --daemon Run as a daemon write PID file.\n
-f | --config file Configuration file.\n
- | --version Display version information.\n
+-V | --version Display version information.\n
-p | --pid-file file Change name of PID file.\n
\n
libvirt lock management daemon:\n), argv0);
@@ -1138,10 +1139,6 @@ virLockDaemonUsage(const char *argv0, bool privileged)
}
}
-enum {
-OPT_VERSION = 129
-};
-
#define
[libvirt] [PATCH 1/2] qemu: Add capability flag for usb-storage
From: Fred A. Kemp ano...@riseup.net
Allow use of the usb-storage device only if the new capability flag
QEMU_CAPS_DEVICE_USB_STORAGE is set, which it is for qemu(-kvm)
versions = 0.12.1.2-rhel62-beta.
---
src/qemu/qemu_capabilities.c |2 ++
src/qemu/qemu_capabilities.h |1 +
src/qemu/qemu_command.c |6 +++---
tests/qemuhelptest.c | 18 --
tests/qemuxml2argvtest.c |3 ++-
5 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 47cc07a..5c566c1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -235,6 +235,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
vnc-share-policy, /* 150 */
device-del-event,
dmi-to-pci-bridge,
+ usb-storage,
);
struct _virQEMUCaps {
@@ -1383,6 +1384,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ vfio-pci, QEMU_CAPS_DEVICE_VFIO_PCI },
{ scsi-generic, QEMU_CAPS_DEVICE_SCSI_GENERIC },
{ i82801b11-bridge, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE },
+{ usb-storage, QEMU_CAPS_DEVICE_USB_STORAGE },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 074e55d..4a8b14b 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -191,6 +191,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */
QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */
+QEMU_CAPS_DEVICE_USB_STORAGE = 153, /* -device usb-storage */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b811e1d..03fb798 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8044,10 +8044,10 @@ qemuBuildCommandLine(virConnectPtr conn,
bool withDeviceArg = false;
bool deviceFlagMasked = false;
-/* Unless we have -device, then USB disks need special
- handling */
+/* Unless we have `-device usb-storage`, then USB disks
+ need special handling */
if ((disk-bus == VIR_DOMAIN_DISK_BUS_USB)
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) {
if (disk-device == VIR_DOMAIN_DISK_DEVICE_DISK) {
virCommandAddArg(cmd, -usbdevice);
virCommandAddArgFormat(cmd, disk:%s, disk-src);
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index d6cc04b..cbabe12 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -514,7 +514,8 @@ mymain(void)
QEMU_CAPS_DEVICE_USB_SERIAL,
QEMU_CAPS_DEVICE_USB_NET,
QEMU_CAPS_DEVICE_PCI_BRIDGE,
-QEMU_CAPS_DEVICE_SCSI_GENERIC);
+QEMU_CAPS_DEVICE_SCSI_GENERIC,
+QEMU_CAPS_DEVICE_USB_STORAGE);
DO_TEST(qemu-kvm-0.12.1.2-rhel61, 12001, 1, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -653,7 +654,8 @@ mymain(void)
QEMU_CAPS_DEVICE_QXL,
QEMU_CAPS_DEVICE_VGA,
QEMU_CAPS_DEVICE_CIRRUS_VGA,
-QEMU_CAPS_DEVICE_PCI_BRIDGE);
+QEMU_CAPS_DEVICE_PCI_BRIDGE,
+QEMU_CAPS_DEVICE_USB_STORAGE);
DO_TEST(qemu-1.0, 100, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -736,7 +738,8 @@ mymain(void)
QEMU_CAPS_DEVICE_USB_SERIAL,
QEMU_CAPS_DEVICE_USB_NET,
QEMU_CAPS_DEVICE_SCSI_GENERIC,
-QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX);
+QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX,
+QEMU_CAPS_DEVICE_USB_STORAGE);
DO_TEST(qemu-1.1.0, 1001000, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -831,7 +834,8 @@ mymain(void)
QEMU_CAPS_DEVICE_PCI_BRIDGE,
QEMU_CAPS_DEVICE_SCSI_GENERIC,
QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX,
-QEMU_CAPS_VNC_SHARE_POLICY);
+QEMU_CAPS_VNC_SHARE_POLICY,
+QEMU_CAPS_DEVICE_USB_STORAGE);
DO_TEST(qemu-1.2.0, 1002000, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -937,7 +941,8 @@ mymain(void)
QEMU_CAPS_DEVICE_PCI_BRIDGE,
QEMU_CAPS_DEVICE_SCSI_GENERIC,
QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX,
-QEMU_CAPS_VNC_SHARE_POLICY);
+QEMU_CAPS_VNC_SHARE_POLICY,
+QEMU_CAPS_DEVICE_USB_STORAGE);
DO_TEST(qemu-kvm-1.2.0, 1002000, 1, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -1048,7 +1053,8 @@ mymain(void)
[libvirt] [PATCH 2/2] qemu: Support setting the 'removable' flag for USB disks
From: Fred A. Kemp ano...@riseup.net
Add an attribute named 'removable' to the 'target' element of disks,
which controls the removable flag. For instance, on a Linux guest it
controls the value of /sys/block/$dev/removable. This option is only
valid for USB disks (i.e. bus='usb'), and its default value is 'off',
which is the same behaviour as before.
To achieve this, 'removable=on' (or 'off') is appended to the '-device
usb-storage' parameter sent to qemu when adding a USB disk via
'-disk'. A capability flag QEMU_CAPS_USB_STORAGE_REMOVABLE was added
to keep track if this option is supported by the qemu version used.
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=922495
---
docs/formatdomain.html.in |8 +++--
docs/schemas/domaincommon.rng |8 +
src/conf/domain_conf.c | 31 ++--
src/conf/domain_conf.h |1 +
src/qemu/qemu_capabilities.c |8 +
src/qemu/qemu_capabilities.h |1 +
src/qemu/qemu_command.c| 17 +++
tests/qemuhelpdata/qemu-1.2.0-device | 11 +++
tests/qemuhelpdata/qemu-kvm-1.2.0-device | 11 +++
tests/qemuhelptest.c |6 ++--
.../qemuxml2argv-disk-usb-device-removable.args|8 +
.../qemuxml2argv-disk-usb-device-removable.xml | 27 +
tests/qemuxml2argvtest.c |3 ++
13 files changed, 133 insertions(+), 7 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index dd22b6d..6fbc418 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1671,9 +1671,13 @@
removable disks (i.e. CDROM or Floppy disk), the value can be either
open or closed, defaults to closed. NB, the value of
codetray/code could be updated while the domain is running.
-span class=sinceSince 0.0.3; codebus/code attribute since
0.4.3;
+The optional attribute coderemovable/code sets the
+removable flag for USB disks, and its value can be either on
+or off, defaulting to off. span class=sinceSince
+0.0.3; codebus/code attribute since 0.4.3;
codetray/code attribute since 0.9.11; usb attribute value since
-after 0.4.4; sata attribute value since 0.9.7/span
+after 0.4.4; sata attribute value since 0.9.7; removable attribute
+value since X.Y.Z/span
/dd
dtcodeiotune/code/dt
ddThe optional codeiotune/code element provides the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ac807e6..aabc592 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1284,6 +1284,14 @@
/choice
/attribute
/optional
+ optional
+attribute name=removable
+ choice
+valueon/value
+valueoff/value
+ /choice
+/attribute
+ /optional
/element
/define
define name=geometry
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7309877..efa19af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4748,6 +4748,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
char *authUUID = NULL;
char *usageType = NULL;
char *tray = NULL;
+char *removable = NULL;
char *logical_block_size = NULL;
char *physical_block_size = NULL;
char *wwn = NULL;
@@ -4904,6 +4905,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
target = virXMLPropString(cur, dev);
bus = virXMLPropString(cur, bus);
tray = virXMLPropString(cur, tray);
+removable = virXMLPropString(cur, removable);
/* HACK: Work around for compat with Xen
* driver in previous libvirt releases */
@@ -5337,6 +5339,24 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
def-tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED;
}
+if (removable) {
+if ((def-removable = virDomainFeatureStateTypeFromString(removable))
0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(unknown disk removable status '%s'), removable);
+goto error;
+}
+
+if (def-bus != VIR_DOMAIN_DISK_BUS_USB) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(removable is only valid for usb disks));
+goto error;
+}
+} else {
+if (def-bus == VIR_DOMAIN_DISK_BUS_USB) {
+def-removable = VIR_DOMAIN_FEATURE_STATE_DEFAULT;
+}
+}
+
if (def-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY
[libvirt] [PATCH 0/2] Support setting the 'removable' flag for USB disks
From: Fred A. Kemp ano...@riseup.net The commit message of patch #2 explains the purpose of this patch set. A review would be greatly appreciated! Note that I've only added the new capability for usb-storage.removable to the qemu help tests of qemu(-kvm) version 1.2.0, since that's what I had easily available to get the output of `-device usb-storage,?` from. I hope that's not an issue, otherwise, is there a way to obtain these outputs without having to hunt down and install all supported versions? Previous submissions of this patch set to this list: http://www.redhat.com/archives/libvir-list/2013-March/msg01051.html http://www.redhat.com/archives/libvir-list/2013-May/msg02039.html https://www.redhat.com/archives/libvir-list/2013-July/msg01635.html Fred A. Kemp (2): qemu: Add capability flag for usb-storage qemu: Support setting the 'removable' flag for USB disks docs/formatdomain.html.in |8 +++-- docs/schemas/domaincommon.rng |8 + src/conf/domain_conf.c | 31 ++-- src/conf/domain_conf.h |1 + src/qemu/qemu_capabilities.c | 10 +++ src/qemu/qemu_capabilities.h |2 ++ src/qemu/qemu_command.c| 23 +-- tests/qemuhelpdata/qemu-1.2.0-device | 11 +++ tests/qemuhelpdata/qemu-kvm-1.2.0-device | 11 +++ tests/qemuhelptest.c | 20 + .../qemuxml2argv-disk-usb-device-removable.args|8 + .../qemuxml2argv-disk-usb-device-removable.xml | 27 + tests/qemuxml2argvtest.c |6 +++- 13 files changed, 151 insertions(+), 15 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Support settings the 'removable' flag for USB disks
Eric Blake wrote: Does this patch still apply as is? No, so I just re-submitted a rebased patch-set: http://www.redhat.com/archives/libvir-list/2013-August/msg00581.html It should be noted that my patches constantly gets in a conflicting state versus your master since they add capabilities, and that seems to be the most rapidly changing place in the code base. Luckily such conflicts are trivial to resolve. In fact, since my second patch submission in May [1], in which I (hope I) fixed all issues found in the review [2] of my first submission in March [3], resolving these types of trivial conflicts is the only thing I've done. [1] http://www.redhat.com/archives/libvir-list/2013-May/msg02039.html [2] http://www.redhat.com/archives/libvir-list/2013-March/msg01065.html [3] http://www.redhat.com/archives/libvir-list/2013-March/msg01051.html I apologize on behalf of a busy team that sometimes patches don't get reviewed, and it is okay to send pings every week or so as needed to spur someone on to looking at it. Ok. Please let me know if there's anything, process-wise, I can do too smoothen this further. Otherwise I'll just continue sending rebased patches as pings every week or two. Cheers! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Link to virdbustest against DBus libs
On Tue, Aug 13, 2013 at 09:51:22AM +0100, Daniel P. Berrange wrote: On Mon, Aug 12, 2013 at 10:13:07PM +0200, Guido Günther wrote: On Mon, Aug 12, 2013 at 01:21:08PM -0600, Eric Blake wrote: On 08/12/2013 01:14 PM, Guido Günther wrote: otherwise we fail like: CCLD virdbustest /usr/bin/ld: virdbustest-virdbustest.o: undefined reference to symbol 'dbus_message_unref' /lib/x86_64-linux-gnu/libdbus-1.so.3: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status Found by: http://honk.sigxcpu.org:8001/job/libvirt-build-debian-sid-amd64/7/console --- tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. I found several more places which fail to link with a strict --no-copy-dt-needed-entries. See my followup patch which superseeds this one. Hmm, it sounds like we ought to make configure.ac automatically add this linker flag if we can detect that it is supported. Does this make sense: Subject: [PATCH] Check for --no-copy-dt-needed linker flag and use it when available --- configure.ac| 1 + m4/virt-linker-relro.m4 | 11 +++ src/Makefile.am | 9 + tests/Makefile.am | 1 + 4 files changed, 22 insertions(+) diff --git a/configure.ac b/configure.ac index ac8cfa1..2084437 100644 --- a/configure.ac +++ b/configure.ac @@ -160,6 +160,7 @@ AC_MSG_RESULT([$VERSION_SCRIPT_FLAGS]) LIBVIRT_COMPILE_WARNINGS LIBVIRT_COMPILE_PIE LIBVIRT_LINKER_RELRO +LIBVIRT_LINKER_NO_COPY_DT_NEEDED_ENTRIES LIBVIRT_CHECK_APPARMOR LIBVIRT_CHECK_ATTR diff --git a/m4/virt-linker-relro.m4 b/m4/virt-linker-relro.m4 index 9bca90e..57b3d03 100644 --- a/m4/virt-linker-relro.m4 +++ b/m4/virt-linker-relro.m4 @@ -30,3 +30,14 @@ AC_DEFUN([LIBVIRT_LINKER_RELRO],[ AC_MSG_RESULT([$RELRO_LDFLAGS]) ]) + +AC_DEFUN([LIBVIRT_LINKER_NO_COPY_DT_NEEDED_ENTRIES],[ +AC_MSG_CHECKING([for how to avoid indirect lib deps]) + +NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS= +`$LD --help 21 | grep -- --no-copy-dt-needed-entries /dev/null` \ +NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS=-Wl,--no-copy-dt-needed-entries +AC_SUBST([NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS]) + +AC_MSG_RESULT([$NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS]) +]) diff --git a/src/Makefile.am b/src/Makefile.am index 4702cde..a04e154 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1813,6 +1813,7 @@ libvirt_la_LDFLAGS = \ $(LIBVIRT_NODELETE) \ $(AM_LDFLAGS) \ $(RELRO_LDFLAGS) \ + $(NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) \ $(NULL) @@ -1897,6 +1898,7 @@ libvirt_qemu_la_LDFLAGS = \ -version-info $(LIBVIRT_VERSION_INFO) \ $(AM_LDFLAGS) \ $(RELRO_LDFLAGS) \ + $(NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) \ $(NULL) @@ -1909,6 +1911,7 @@ libvirt_lxc_la_LDFLAGS = \ -version-info $(LIBVIRT_VERSION_INFO) \ $(AM_LDFLAGS) \ $(RELRO_LDFLAGS) \ + $(NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) \ $(NULL) @@ -1965,6 +1968,7 @@ virtlockd_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ + $(NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) \ $(NULL) @@ -2244,6 +2248,7 @@ libvirt_iohelper_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ + $(NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS) \ $(NULL) libvirt_iohelper_LDADD = \ libvirt_util.la \ @@ -2267,6 +2272,7 @@ libvirt_parthelper_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ + $(NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS) \ $(NULL) libvirt_parthelper_LDADD = \ $(LIBPARTED_LIBS) \ @@ -2299,6 +2305,7 @@ libvirt_sanlock_helper_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ + $(NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS) \ $(NULL) libvirt_sanlock_helper_LDADD = libvirt.la endif @@ -2315,6 +2322,7 @@ libvirt_lxc_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ + $(NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS) \ $(NULL) libvirt_lxc_LDADD =\ $(FUSE_LIBS) \ @@ -2359,6 +2367,7 @@
Re: [libvirt] [PATCH] virsh-domain: Flip logic in cmdSetvcpus
On 08/13/13 12:03, Ján Tomko wrote: On 08/13/2013 11:22 AM, Peter Krempa wrote: To avoid having to assign a failure code to the returned variable switch this function to negative logic. This will fix issue with invalid number of cpus returning success return code. https://bugzilla.redhat.com/show_bug.cgi?id=996466 --- tools/virsh-domain.c | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) ACK, nice reduction. Jan Pushed. Thanks. Peter 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] Directly link against needed libraries
On 08/12/2013 11:54 PM, Guido Günther wrote: The Linux build revealed another missing direkt link target, this time s/direkt/direct/ against selinux libs: http://honk.sigxcpu.org:8001/view/libvirt/job/libvirt-build-debian-sid-amd64/9/console --- Sorry for missing this in the previous patch. -- GUido tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 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
[libvirt] [PATCH] Honour root prefix in lxcContainerMountFSBlockAuto
From: Daniel P. Berrange berra...@redhat.com
The lxcContainerMountFSBlockAuto method can be used to mount the
initial root filesystem, so it cannot assume a prefix of /.oldroot.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_container.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index a943b22..0ab4026 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1144,7 +1144,8 @@ lxcContainerMountDetectFilesystem(const char *src
ATTRIBUTE_UNUSED,
*/
static int lxcContainerMountFSBlockAuto(virDomainFSDefPtr fs,
int fsflags,
-const char *src)
+const char *src,
+const char *srcprefix)
{
FILE *fp = NULL;
int ret = -1;
@@ -1154,11 +1155,11 @@ static int
lxcContainerMountFSBlockAuto(virDomainFSDefPtr fs,
char *line = NULL;
const char *type;
-VIR_DEBUG(src=%s dst=%s, src, fs-dst);
+VIR_DEBUG(src=%s dst=%s srcprefix=%s, src, fs-dst, srcprefix);
/* First time around we use /etc/filesystems */
retry:
-if (virAsprintf(fslist, /.oldroot%s,
+if (virAsprintf(fslist, %s%s, srcprefix,
tryProc ? /proc/filesystems : /etc/filesystems) 0)
goto cleanup;
@@ -1270,7 +1271,8 @@ cleanup:
* probing for filesystem type
*/
static int lxcContainerMountFSBlockHelper(virDomainFSDefPtr fs,
- const char *src)
+ const char *src,
+ const char *srcprefix)
{
int fsflags = 0;
int ret = -1;
@@ -1300,7 +1302,7 @@ static int
lxcContainerMountFSBlockHelper(virDomainFSDefPtr fs,
}
ret = 0;
} else {
-ret = lxcContainerMountFSBlockAuto(fs, fsflags, src);
+ret = lxcContainerMountFSBlockAuto(fs, fsflags, src, srcprefix);
}
cleanup:
@@ -1318,7 +1320,7 @@ static int lxcContainerMountFSBlock(virDomainFSDefPtr fs,
if (virAsprintf(src, %s%s, srcprefix, fs-src) 0)
goto cleanup;
-ret = lxcContainerMountFSBlockHelper(fs, src);
+ret = lxcContainerMountFSBlockHelper(fs, src, srcprefix);
VIR_DEBUG(Done mounting filesystem ret=%d, ret);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Address missed feedback from review of virt-login-shell
On 08/13/2013 05:16 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Address a number of code, style and docs issues identified
in review of virt-login-shell after it was merged.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tools/Makefile.am | 1 -
tools/virt-login-shell.c | 58
++
tools/virt-login-shell.pod | 30 ++--
3 files changed, 61 insertions(+), 28 deletions(-)
ACK.
@@ -327,7 +345,7 @@ main(int argc, char **argv)
}
if (execv(shargv[0], (char *const*) shargv) 0) {
virReportSystemError(errno, _(Unable exec shell %s),
shargv[0]);
-return -errno;
+return EXIT_FAILURE;
Setting $? to 1 works, although it's more typical to set to 126 or 127
on execv failure. But I'm fine with it as-is.
--
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] Link to virdbustest against DBus libs
On Tue, Aug 13, 2013 at 02:23:41PM +0200, Guido Günther wrote: On Tue, Aug 13, 2013 at 09:51:22AM +0100, Daniel P. Berrange wrote: On Mon, Aug 12, 2013 at 10:13:07PM +0200, Guido Günther wrote: On Mon, Aug 12, 2013 at 01:21:08PM -0600, Eric Blake wrote: On 08/12/2013 01:14 PM, Guido Günther wrote: otherwise we fail like: CCLD virdbustest /usr/bin/ld: virdbustest-virdbustest.o: undefined reference to symbol 'dbus_message_unref' /lib/x86_64-linux-gnu/libdbus-1.so.3: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status Found by: http://honk.sigxcpu.org:8001/job/libvirt-build-debian-sid-amd64/7/console --- tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. I found several more places which fail to link with a strict --no-copy-dt-needed-entries. See my followup patch which superseeds this one. Hmm, it sounds like we ought to make configure.ac automatically add this linker flag if we can detect that it is supported. Does this make sense: Subject: [PATCH] Check for --no-copy-dt-needed linker flag and use it when available --- configure.ac| 1 + m4/virt-linker-relro.m4 | 11 +++ src/Makefile.am | 9 + tests/Makefile.am | 1 + 4 files changed, 22 insertions(+) diff --git a/configure.ac b/configure.ac index ac8cfa1..2084437 100644 --- a/configure.ac +++ b/configure.ac @@ -160,6 +160,7 @@ AC_MSG_RESULT([$VERSION_SCRIPT_FLAGS]) LIBVIRT_COMPILE_WARNINGS LIBVIRT_COMPILE_PIE LIBVIRT_LINKER_RELRO +LIBVIRT_LINKER_NO_COPY_DT_NEEDED_ENTRIES LIBVIRT_CHECK_APPARMOR LIBVIRT_CHECK_ATTR diff --git a/m4/virt-linker-relro.m4 b/m4/virt-linker-relro.m4 index 9bca90e..57b3d03 100644 --- a/m4/virt-linker-relro.m4 +++ b/m4/virt-linker-relro.m4 @@ -30,3 +30,14 @@ AC_DEFUN([LIBVIRT_LINKER_RELRO],[ AC_MSG_RESULT([$RELRO_LDFLAGS]) ]) + +AC_DEFUN([LIBVIRT_LINKER_NO_COPY_DT_NEEDED_ENTRIES],[ +AC_MSG_CHECKING([for how to avoid indirect lib deps]) + +NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS= +`$LD --help 21 | grep -- --no-copy-dt-needed-entries /dev/null` \ +NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS=-Wl,--no-copy-dt-needed-entries +AC_SUBST([NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS]) + +AC_MSG_RESULT([$NO_COPY_DT_NEEDED_ENTRIES_LDFLAGS]) +]) This doesn't belong in this file. Create a new virt-linker-no-indirect.m4 file for it. The variable / macro names are rather too long too - use something like NO_INDIRECT_LDFLAGS / LIBVIRT_LINKER_NO_INDIRECT respectively. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Properly handle -h / -V for --help/--version aliases in virtlockd/libvirtd
On 08/13/2013 05:17 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The virtlockd/libvirtd daemons had listed '?' as the short option
for --help. getopt_long uses '?' for any unknown option. We want
to be able to distinguish unknown options (which use EXIT_FAILURE)
from correct usage of help (which should use EXIT_SUCCESS). Thus
we should use 'h' as a short option for --help. Also add this to
the man page docs
The virtlockd/libvirtd daemons did not list any short option
for the --version arg. Add -V as a valid short option, since
-v is already used for --verbose.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
daemon/libvirtd.c| 31 ++-
daemon/libvirtd.pod.in | 4
src/locking/lock_daemon.c| 29 +
src/locking/virtlockd.pod.in | 6 +-
4 files changed, 36 insertions(+), 34 deletions(-)
ACK.
-enum {
-OPT_VERSION = 129
-};
Good riddance - on single-byte locales, it is actually possible to type
byte 129 (when using a long-only option, you should generally list it
starting from 257, so that it can't be typed as an accidental short
option of some weird character).
--
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] [PATCH] doc: storage pool permission copy-paste fix
The description for permissions was copied from the storage volume section to the storage pool section, but the semantics are different: 1. Currently only the dir, fs and netfs storage pools use it. 2. They use it only to build the final directory. 3. A default for the storage volumes can't be set. Signed-off-by: Philipp Hahn h...@univention.de --- docs/formatstorage.html.in |9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index f4d561f..1fcfb6b 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -214,11 +214,10 @@ span class=sinceSince 0.4.1/span /dd dtcodepermissions/code/dt - ddProvides information about the default permissions to use -when creating volumes. This is currently only useful for directory -or filesystem based pools, where the volumes allocated are simple -files. For pools where the volumes are device nodes, the hotplug -scripts determine permissions. It contains 4 child elements. The + ddThis is currently only useful for directory or filesystem based +pools, which are mapped as a directory into the local filesystem +namespace. It provides information about the permissions to use for the +final directory when the pool is built. The codemode/code element contains the octal permission set. The codeowner/code element contains the numeric user ID. The codegroup/code element contains the numeric group ID. The codelabel/code element -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Honour root prefix in lxcContainerMountFSBlockAuto
On 08/13/2013 06:26 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The lxcContainerMountFSBlockAuto method can be used to mount the initial root filesystem, so it cannot assume a prefix of /.oldroot. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/lxc/lxc_container.c | 14 -- 1 file changed, 8 insertions(+), 6 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
[libvirt] [PATCH] Ensure that /dev exists in the container root filesystem
From: Daniel P. Berrange berra...@redhat.com
If booting a container with a root FS that isn't the host's
root, we must ensure that the /dev mount point exists.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_container.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 0ab4026..678a9d5 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -868,7 +868,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def
ATTRIBUTE_UNUSED,
static int lxcContainerMountFSDev(virDomainDefPtr def,
const char *stateDir)
{
-int ret;
+int ret = -1;
char *path = NULL;
VIR_DEBUG(Mount /dev/ stateDir=%s, stateDir);
@@ -877,14 +877,24 @@ static int lxcContainerMountFSDev(virDomainDefPtr def,
stateDir, def-name)) 0)
return ret;
+if (virFileMakePath(/dev) 0) {
+virReportSystemError(errno, %s,
+ _(Cannot create /dev));
+goto cleanup;
+}
+
VIR_DEBUG(Tring to move %s to /dev, path);
-if ((ret = mount(path, /dev, NULL, MS_MOVE, NULL)) 0) {
+if (mount(path, /dev, NULL, MS_MOVE, NULL) 0) {
virReportSystemError(errno,
_(Failed to mount %s on /dev),
path);
+goto cleanup;
}
+ret = 0;
+
+cleanup:
VIR_FREE(path);
return ret;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] cpu: Add Power7+ and Power8 CPU definition in map.xml
On Tue, Aug 13, 2013 at 11:55:40AM +0800, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com Power7+ and Power8 are supported in QEMU, so it needs to define CPUs in libvirt to support them. Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- src/cpu/cpu_map.xml | 11 +++ 1 file changed, 11 insertions(+) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 6d51283..7d34d40 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -603,5 +603,16 @@ vendor name='IBM'/ pvr value='0x003f0203'/ /model + +model name='POWER7+_v2.1' + vendor name='IBM'/ + pvr value='0x004a0201'/ +/model + +model name='POWER8_v1.0' + vendor name='IBM'/ + pvr value='0x004b0100'/ +/model + /arch /cpus ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] selinux: distinguish failure to label from request to avoid label
On Mon, Aug 12, 2013 at 10:19:47PM -0600, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=924153 Commit 904e05a2 (v0.9.9) added a per-disk seclabel element with an attribute relabel='no' in order to try and minimize the impact of shutdown delays when an NFS server disappears. The idea was that if a disk is on NFS and can't be labeled in the first place, there is no need to attempt the (no-op) relabel on domain shutdown. Unfortunately, the way this was implemented was by modifying the domain XML so that the optimization would survive libvirtd restart, but in a way that is indistinguishable from an explicit user setting. Furthermore, once the setting is turned on, libvirt avoids attempts at labeling, even for operations like snapshot or blockcopy where the chain is being extended or pivoted onto non-NFS, where SELinux labeling is once again possible. As a result, it was impossible to do a blockcopy to pivot from an NFS image file onto a local file. The solution is to separate the semantics of a chain that must not be labeled (which the user can set even on persistent domains) vs. the optimization of not attempting a relabel on cleanup (a live-only annotation), and using only the user's explicit notation rather than the optimization as the decision on whether to skip a label attempt in the first place. When upgrading an older libvirtd to a newer, an NFS volume will still attempt the relabel; but as the avoidance of a relabel was only an optimization, this shouldn't cause any problems. In the ideal future, libvirt will eventually have XML describing EVERY file in the backing chain, with each file having a separate seclabel element. At that point, libvirt will be able to track more closely which files need a relabel attempt at shutdown. But until we reach that point, the single seclabel for the entire disk chain is treated as a hint - when a chain has only one file, then we know it is accurate; but if the chain has more than one file, we have to attempt relabel in spite of the attribute, in case part of the chain is local and SELinux mattered for that portion of the chain. * src/conf/domain_conf.h (_virSecurityDeviceLabelDef): Add new member. * src/conf/domain_conf.c (virSecurityDeviceLabelDefParseXML): Parse it, for live images only. (virSecurityDeviceLabelDefFormat): Output it. (virDomainDiskDefParseXML, virDomainChrSourceDefParseXML) (virDomainDiskSourceDefFormat, virDomainChrDefFormat) (virDomainDiskDefFormat): Pass flags on through. * src/security/security_selinux.c (virSecuritySELinuxRestoreSecurityImageLabelInt): Honor labelskip when possible. (virSecuritySELinuxSetSecurityFileLabel): Set labelskip, not norelabel, if labeling fails. * docs/formatdomain.html.in (seclabel): Document new xml. * docs/schemas/domaincommon.rng (devSeclabel): Allow it in RNG. * tests/qemuxml2argvdata/qemuxml2argv-seclabel-*-skiplabel.xml: * tests/qemuxml2argvdata/qemuxml2argv-seclabel-*-skiplabel.args: * tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-*-skiplabel.xml: New test files. * tests/qemuxml2argvtest.c (mymain): Run the new tests. * tests/qemuxml2xmltest.c (mymain): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- docs/formatdomain.html.in | 6 ++- docs/schemas/domaincommon.rng | 27 +++-- src/conf/domain_conf.c | 47 -- src/conf/domain_conf.h | 3 +- src/security/security_selinux.c| 10 - .../qemuxml2argv-seclabel-dynamic-skiplabel.args | 5 +++ .../qemuxml2argv-seclabel-dynamic-skiplabel.xml| 32 +++ .../qemuxml2argv-seclabel-static-skiplabel.args| 5 +++ .../qemuxml2argv-seclabel-static-skiplabel.xml | 33 +++ tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-seclabel-dynamic-skiplabel.xml | 31 ++ tests/qemuxml2xmltest.c| 8 ++-- 12 files changed, 178 insertions(+), 31 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-skiplabel.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-skiplabel.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-static-skiplabel.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-static-skiplabel.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-skiplabel.xml The changes look reasonable, but I'd be alot happier if the securityselinuxlabeltest.c was covering this scenario. We already have that test using an LD_PRELOAD hack to mock the selinux APIs. It ought to be possible to extend it to return the same errno conditions you'd see on NFS, when given certain filenames, to allow this code to be validated. Regards, Daniel -- |: http://berrange.com -o-
Re: [libvirt] [PATCH 1/2] qemu: Add capability flag for usb-storage
On Tue, Aug 13, 2013 at 01:52:33PM +0200, Fred A. Kemp wrote:
From: Fred A. Kemp ano...@riseup.net
Allow use of the usb-storage device only if the new capability flag
QEMU_CAPS_DEVICE_USB_STORAGE is set, which it is for qemu(-kvm)
versions = 0.12.1.2-rhel62-beta.
---
src/qemu/qemu_capabilities.c |2 ++
src/qemu/qemu_capabilities.h |1 +
src/qemu/qemu_command.c |6 +++---
tests/qemuhelptest.c | 18 --
tests/qemuxml2argvtest.c |3 ++-
5 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 47cc07a..5c566c1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -235,6 +235,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
vnc-share-policy, /* 150 */
device-del-event,
dmi-to-pci-bridge,
+ usb-storage,
);
struct _virQEMUCaps {
@@ -1383,6 +1384,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[]
= {
{ vfio-pci, QEMU_CAPS_DEVICE_VFIO_PCI },
{ scsi-generic, QEMU_CAPS_DEVICE_SCSI_GENERIC },
{ i82801b11-bridge, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE },
+{ usb-storage, QEMU_CAPS_DEVICE_USB_STORAGE },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 074e55d..4a8b14b 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -191,6 +191,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */
QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */
+QEMU_CAPS_DEVICE_USB_STORAGE = 153, /* -device usb-storage */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b811e1d..03fb798 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8044,10 +8044,10 @@ qemuBuildCommandLine(virConnectPtr conn,
bool withDeviceArg = false;
bool deviceFlagMasked = false;
-/* Unless we have -device, then USB disks need special
- handling */
+/* Unless we have `-device usb-storage`, then USB disks
+ need special handling */
if ((disk-bus == VIR_DOMAIN_DISK_BUS_USB)
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) {
if (disk-device == VIR_DOMAIN_DISK_DEVICE_DISK) {
virCommandAddArg(cmd, -usbdevice);
virCommandAddArgFormat(cmd, disk:%s, disk-src);
Hmm, I'm not sure this logic change is right.
Previously if you have -device support, but 'usb-storage' was not
available, libvirt would try to start the guest with -device then
QEMU would report an error.
With this change, if you have -device support, but 'usb-storage' was
not available, then libvirt is going to fallback to using the legacy
'-usbdevice' support. This is not good.
Adding an explicit check for 'usb-storage' is a fine goal, but we
should be doing that check in the branch of this if() that handles
'-device usb-storage', so we don't exercise the -usbdevice branch
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH][Ruby] support: virNetworkUpdate
Hi Chris, On 2013年08月13日 09:55, Chris Lalancette wrote: Thanks for the patch, that is great! I've now applied it to the main ruby-libvirt repository. Let me know if you have any problems with it. Looks good. No problem. IMHO, there are many progress from 0.4.0 release, it is chance to release 0.5.0 :-) There is a small problem that a commit in 2008 had a wrong date record. (That makes warn when pushing it on github.com) https://github.com/miurahr/ruby-libvirt/compare/master...fixInvalidCommit Hiroshi -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ensure that /dev exists in the container root filesystem
On 08/13/2013 07:59 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com If booting a container with a root FS that isn't the host's root, we must ensure that the /dev mount point exists. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/lxc/lxc_container.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) ACK. + VIR_DEBUG(Tring to move %s to /dev, path); While you're touching this, s/Tring/Trying/ -- 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] selinux: distinguish failure to label from request to avoid label
On 08/13/2013 08:11 AM, Daniel P. Berrange wrote: On Mon, Aug 12, 2013 at 10:19:47PM -0600, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=924153 Commit 904e05a2 (v0.9.9) added a per-disk seclabel element with an attribute relabel='no' in order to try and minimize the impact of shutdown delays when an NFS server disappears. The idea was that if a disk is on NFS and can't be labeled in the first place, there is no need to attempt the (no-op) relabel on domain shutdown. Unfortunately, the way this was implemented was by modifying the domain XML so that the optimization would survive libvirtd restart, but in a way that is indistinguishable from an explicit user setting. Furthermore, once the setting is turned on, libvirt avoids attempts at labeling, even for operations like snapshot or blockcopy where the chain is being extended or pivoted onto non-NFS, where SELinux labeling is once again possible. As a result, it was impossible to do a blockcopy to pivot from an NFS image file onto a local file. The changes look reasonable, but I'd be alot happier if the securityselinuxlabeltest.c was covering this scenario. We already have that test using an LD_PRELOAD hack to mock the selinux APIs. It ought to be possible to extend it to return the same errno conditions you'd see on NFS, when given certain filenames, to allow this code to be validated. Okay, I'll work on a followup patch to do that. -- 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] Directly link against needed libraries
On Tue, Aug 13, 2013 at 06:29:10AM -0600, Eric Blake wrote: On 08/12/2013 11:54 PM, Guido Günther wrote: The Linux build revealed another missing direkt link target, this time s/direkt/direct/ Fixed and pushed. Thanks. -- Guido against selinux libs: http://honk.sigxcpu.org:8001/view/libvirt/job/libvirt-build-debian-sid-amd64/9/console --- Sorry for missing this in the previous patch. -- GUido tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Add virt-sandbox -s inherit, to execute the sandbox with parents label
This will allow us to run sandbox as the calling process, If I am running a shell as staff_u:unconfined_r:unconfined_t:s0, and I execute virt-sandbox -c lxc/// -- /bin/sh The second patch fixes a problem when users try to upgrade Generic Containers. [sandbox PATCH 1/2] Add virt-sandbox -s inherit, to execute the [sandbox PATCH 2/2] GenericContainers do not have unit files. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Updated patch for virt-sandbox -s inherit
-s static,label=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 Well running virt-sandbox -s inherit would run as unconfined_t for most users. I the future we need to add a check to libvirt to ask SELinux if it is ok for a user to transiton to the label, rather then just to do it. Imagine a confined admin which is allowed to generate containers, he should only be allowed to generate containers with processes labels that he can transition into, not that libvirt can transition into. [sandbox PATCH 1/2] Add virt-sandbox -s inherit, to execute the [sandbox PATCH 2/2] Unit files only exist in Systemd Containers. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 1/2] Add virt-sandbox -s inherit, to execute the sandbox from the parent.
This will allow us to run sandbox as the calling process, If I am
running a shell as staff_u:unconfined_r:unconfined_t:s0, and I
execute virt-sandbox -c lxc/// -- /bin/sh
/bin/sh will run as staff_u:unconfined_r:unconfined_t:s0
---
bin/virt-sandbox-service.pod | 6 +-
bin/virt-sandbox.c | 9 -
configure.ac | 1 +
libvirt-sandbox.spec.in | 1 +
libvirt-sandbox/Makefile.am | 2 ++
libvirt-sandbox/libvirt-sandbox-config.c | 14 ++
m4/virt-selinux.m4 | 11 +++
7 files changed, 42 insertions(+), 2 deletions(-)
create mode 100644 m4/virt-selinux.m4
diff --git a/bin/virt-sandbox-service.pod b/bin/virt-sandbox-service.pod
index 7752145..b879a46 100644
--- a/bin/virt-sandbox-service.pod
+++ b/bin/virt-sandbox-service.pod
@@ -54,7 +54,11 @@ supported currently).
=head1 SEE ALSO
-Clibvirt(8), Cselinux(8), Csystemd(8), Cvirt-sandbox(1),
Cvirt-sandbox-service-create(1), Cvirt-sandbox-service-clone(1),
Cvirt-sandbox-service-connect(1), Cvirt-sandbox-service-delete(1),
Cvirt-sandbox-service-execute(1), Cvirt-sandbox-service-reload(1),
Cvirt-sandbox-service-upgrade(1)
+Clibvirt(8), Cselinux(8), Csystemd(8), Cvirt-sandbox(1),
+Cvirt-sandbox-service-create(1), Cvirt-sandbox-service-clone(1),
+Cvirt-sandbox-service-connect(1), Cvirt-sandbox-service-delete(1),
+Cvirt-sandbox-service-execute(1), Cvirt-sandbox-service-reload(1),
+Cvirt-sandbox-service-upgrade(1)
=head1 FILES
diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c
index 3ddcd17..1132c09 100644
--- a/bin/virt-sandbox.c
+++ b/bin/virt-sandbox.c
@@ -285,7 +285,10 @@ not allowed to open any other files.
=item B-c URI, B--connect=URI
Set the libvirt connection URI, defaults to qemu:///session if
-omitted. Currently only the QEMU and LXC drivers are supported.
+omitted. Alternatively the CLIBVIRT_DEFAULT_URI environment
+variable can be set, or the config file C/etc/libvirt/libvirt.conf
+can have a default URI set. Currently only the QEMU and LXC drivers
+are supported.
=item B-n NAME, B--name=NAME
@@ -417,6 +420,10 @@ USER:ROLE:TYPE:LEVEL, instead of the default base context.
To set a completely static label. For example,
static,label=system_u:system_r:svirt_t:s0:c412,c355
+=item inherit
+
+Inherit the context from the process that is executing virt-sandbox.
+
=back
=item B-p, B--privileged
diff --git a/configure.ac b/configure.ac
index 32206b8..50f23fc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -84,6 +84,7 @@ LIBVIRT_SANDBOX_WIN32
LIBVIRT_SANDBOX_COVERAGE
LIBVIRT_SANDBOX_INTROSPECTION
LIBVIRT_SANDBOX_RPCGEN
+LIBVIRT_SANDBOX_SELINUX
dnl Should be in m4/virt-gettext.m4 but intltoolize is too
dnl dumb to find it there
diff --git a/libvirt-sandbox.spec.in b/libvirt-sandbox.spec.in
index a9721b5..718c27b 100644
--- a/libvirt-sandbox.spec.in
+++ b/libvirt-sandbox.spec.in
@@ -25,6 +25,7 @@ BuildRequires: gobject-introspection-devel
BuildRequires: glibc-static
BuildRequires: /usr/bin/pod2man
BuildRequires: intltool
+BuildRequires: libselinux-devel
BuildRequires: glib2-devel = 2.32.0
Requires: rpm-python
# For virsh lxc-enter-namespace command
diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am
index 4e0ea00..0882490 100644
--- a/libvirt-sandbox/Makefile.am
+++ b/libvirt-sandbox/Makefile.am
@@ -169,6 +169,7 @@ libvirt_sandbox_init_common_CFLAGS = \
$(LIBVIRT_GLIB_CFLAGS) \
$(LIBVIRT_GOBJECT_CFLAGS) \
$(CAPNG_CFLAGS) \
+ $(SELINUX_CFLAGS) \
$(WARN_CFLAGS) \
$(NULL)
libvirt_sandbox_init_common_LDFLAGS = \
@@ -178,6 +179,7 @@ libvirt_sandbox_init_common_LDFLAGS = \
$(LIBVIRT_GLIB_LIBS) \
$(LIBVIRT_GOBJECT_LIBS) \
$(CAPNG_LIBS) \
+ $(SELINUX_LIBS) \
$(WARN_CFLAGS) \
$(NULL)
libvirt_sandbox_init_common_LDADD = \
diff --git a/libvirt-sandbox/libvirt-sandbox-config.c
b/libvirt-sandbox/libvirt-sandbox-config.c
index ccdb3bc..8e8ac65 100644
--- a/libvirt-sandbox/libvirt-sandbox-config.c
+++ b/libvirt-sandbox/libvirt-sandbox-config.c
@@ -27,6 +27,8 @@
#include glib/gi18n.h
#include libvirt-sandbox/libvirt-sandbox.h
+#include errno.h
+#include selinux/selinux.h
/**
* SECTION: libvirt-sandbox-config
@@ -1521,6 +1523,18 @@ gboolean
gvir_sandbox_config_set_security_opts(GVirSandboxConfig *config,
gvir_sandbox_config_set_security_dynamic(config, TRUE);
} else if (g_str_equal(tmp, static)) {
gvir_sandbox_config_set_security_dynamic(config, FALSE);
+} else if (g_str_equal(tmp, inherit)) {
+gvir_sandbox_config_set_security_dynamic(config, FALSE);
+security_context_t scon;
+if (getcon(scon) 0) {
+g_set_error(error,
[libvirt] [sandbox PATCH 2/2] Unit files only exist in Systemd Containers.
Do not attempt to fix the unit file of Generic Containers. --- bin/virt-sandbox-service | 36 ++-- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index 03873c9..3e83c94 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -928,28 +928,28 @@ def upgrade_config_legacy(path): else: container = SystemdContainer(uri=args.uri, config=config) -fd = open(container.get_unit_path()) -unitfile = fd.read() -fd.close() +fd = open(container.get_unit_path()) +unitfile = fd.read() +fd.close() -unitfile = unitfile.replace(/usr/bin/virt-sandbox-service start, -/usr/libexec/virt-sandbox-service-util -c lxc:/// -s) -unitfile = unitfile.replace(/usr/bin/virt-sandbox-service reload, -/usr/bin/virt-sandbox-service -c lxc:/// reload) -unitfile = unitfile.replace(/usr/bin/virt-sandbox-service stop, -/usr/bin/virsh -c lxc:/// destroy) +unitfile = unitfile.replace(/usr/bin/virt-sandbox-service start, +/usr/libexec/virt-sandbox-service-util -c lxc:/// -s) +unitfile = unitfile.replace(/usr/bin/virt-sandbox-service reload, +/usr/bin/virt-sandbox-service -c lxc:/// reload) +unitfile = unitfile.replace(/usr/bin/virt-sandbox-service stop, +/usr/bin/virsh -c lxc:/// destroy) -unitfile = re.sub(WantedBy=.*\.target, - WantedBy=multi-user.target, - unitfile) +unitfile = re.sub(WantedBy=.*\.target, + WantedBy=multi-user.target, + unitfile) -os.remove(container.get_unit_path()) -fd = open(container.get_unit_path(), wx) -fd.write(unitfile) -fd.close() +os.remove(container.get_unit_path()) +fd = open(container.get_unit_path(), wx) +fd.write(unitfile) +fd.close() -sys.stdout.write(_(Created unit file %s\n) % - container.get_unit_path()) +sys.stdout.write(_(Created unit file %s\n) % + container.get_unit_path()) # Create new config file + libvirt persistent XML config container.save_config() -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Address missed feedback from review of virt-login-shell
On Tue, Aug 13, 2013 at 1:16 PM, Daniel P. Berrange berra...@redhat.comwrote: virReportSystemError(errno, _(Unable exec shell %s), shargv[0]); s/Unable/Unable to/ Kind regards, Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/1] selinux: enhance test to cover nfs label failure
Daniel Berrange (correctly) pointed out that we should do a better
job of testing selinux labeling fallbacks on NFS disks that lack
labeling support.
* tests/securityselinuxhelper.c (includes): Makefile already
guaranteed xattr support. Add additional headers.
(init_syms): New function, borrowing from vircgroupmock.c.
(setfilecon_raw, getfilecon_raw): Fake NFS failure.
(statfs): Fake an NFS mount point.
(security_getenforce, security_get_boolean_active): Don't let host
environment affect test.
* tests/securityselinuxlabeldata/nfs.data: New file.
* tests/securityselinuxlabeldata/nfs.xml: New file.
* tests/securityselinuxlabeltest.c (testSELinuxCreateDisks)
(testSELinuxDeleteDisks): Setup and cleanup for fake NFS mount.
(testSELinuxCheckLabels): Test handling of SELinux NFS denial.
Fix memory leak.
(testSELinuxLabeling): Avoid infinite loop on dirty tree.
(mymain): Add new test.
---
tests/securityselinuxhelper.c | 84 ++
tests/securityselinuxlabeldata/nfs.txt | 1 +
tests/securityselinuxlabeldata/nfs.xml | 24 ++
tests/securityselinuxlabeltest.c | 17 +--
4 files changed, 115 insertions(+), 11 deletions(-)
create mode 100644 tests/securityselinuxlabeldata/nfs.txt
create mode 100644 tests/securityselinuxlabeldata/nfs.xml
diff --git a/tests/securityselinuxhelper.c b/tests/securityselinuxhelper.c
index a82ca6d..d7aae26 100644
--- a/tests/securityselinuxhelper.c
+++ b/tests/securityselinuxhelper.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2012 Red Hat, Inc.
+ * Copyright (C) 2011-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -19,22 +19,51 @@
#include config.h
+/* This file is only compiled on Linux, and only if xattr support was
+ * detected. */
+
+#include dlfcn.h
+#include errno.h
+#include linux/magic.h
#include selinux/selinux.h
+#include stdio.h
#include stdlib.h
#include string.h
+#include sys/vfs.h
#include unistd.h
-#include errno.h
-#if WITH_ATTR
-# include attr/xattr.h
-#endif
+#include attr/xattr.h
#include virstring.h
+static int (*realstatfs)(const char *path, struct statfs *buf);
+static int (*realsecurity_get_boolean_active)(const char *name);
+
+static void init_syms(void)
+{
+if (realstatfs)
+return;
+
+# define LOAD_SYM(name) \
+do {\
+if (!(real ## name = dlsym(RTLD_NEXT, #name))) {\
+fprintf(stderr, Cannot find real '%s' symbol\n, #name); \
+abort();\
+} \
+} while (0)
+
+LOAD_SYM(statfs);
+LOAD_SYM(security_get_boolean_active);
+}
+
+
/*
* The kernel policy will not allow us to arbitrarily change
* test process context. This helper is used as an LD_PRELOAD
* so that the libvirt code /thinks/ it is changing/reading
- * the process context, where as in fact we're faking it all
+ * the process context, where as in fact we're faking it all.
+ * Furthermore, we fake out that we are using an nfs subdirectory,
+ * where we control whether selinux is enforcing and whether
+ * the virt_use_nfs bool is set.
*/
int getcon_raw(security_context_t *context)
@@ -83,10 +112,13 @@ int setcon(security_context_t context)
}
-#if WITH_ATTR
int setfilecon_raw(const char *path, security_context_t con)
{
const char *constr = con;
+if (STRPREFIX(path, abs_builddir /securityselinuxlabeldata/nfs/)) {
+errno = EOPNOTSUPP;
+return -1;
+}
return setxattr(path, user.libvirt.selinux,
constr, strlen(constr), 0);
}
@@ -101,6 +133,10 @@ int getfilecon_raw(const char *path, security_context_t
*con)
char *constr = NULL;
ssize_t len = getxattr(path, user.libvirt.selinux,
NULL, 0);
+if (STRPREFIX(path, abs_builddir /securityselinuxlabeldata/nfs/)) {
+errno = EOPNOTSUPP;
+return -1;
+}
if (len 0)
return -1;
if (!(constr = malloc(len+1)))
@@ -114,8 +150,40 @@ int getfilecon_raw(const char *path, security_context_t
*con)
constr[len] = '\0';
return 0;
}
+
+
int getfilecon(const char *path, security_context_t *con)
{
return getfilecon_raw(path, con);
}
-#endif
+
+
+int statfs(const char *path, struct statfs *buf)
+{
+int ret;
+
+init_syms();
+
+ret = realstatfs(path, buf);
+if (!ret STREQ(path, abs_builddir /securityselinuxlabeldata/nfs))
+buf-f_type = NFS_SUPER_MAGIC;
+return ret;
+}
+
+
+int security_getenforce(void)
+{
+/* For the purpose of our test, we are enforcing. */
+return 1;
+}
+
+
+int security_get_boolean_active(const char *name)
+{
+/* For the purpose of our test, nfs is not permitted. */
+if (STREQ(name,
Re: [libvirt] [PATCH] doc: storage pool permission copy-paste fix
On 08/13/2013 06:38 AM, Philipp Hahn wrote: The description for permissions was copied from the storage volume section to the storage pool section, but the semantics are different: 1. Currently only the dir, fs and netfs storage pools use it. 2. They use it only to build the final directory. 3. A default for the storage volumes can't be set. Signed-off-by: Philipp Hahn h...@univention.de --- docs/formatstorage.html.in |9 - 1 file changed, 4 insertions(+), 5 deletions(-) ACK and 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
Re: [libvirt] [PATCH 1/1] cpu: Add Power7+ and Power8 CPU definition in map.xml
On 08/13/2013 08:05 AM, Daniel P. Berrange wrote: On Tue, Aug 13, 2013 at 11:55:40AM +0800, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com Power7+ and Power8 are supported in QEMU, so it needs to define CPUs in libvirt to support them. ACK 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
Re: [libvirt] RFC: Introduce API to return configuration/state paths of the network driver
On 07/25/2013 03:35 AM, Daniel P. Berrange wrote: NACK, As I explained on IRC, the hypervisor drivers have no business accessing the dnsmasq lease files from the bridge driver. This is considered to be a private implementation detail. At a conceptual level, what you're after here is a list of all the IP, mac address mappings of the virtual network. This information is useful even outside the context of the hypervisor driver method you're working on. So we should create formal APIs for exposing this, something like: virNetworkGetDHCPLeases(virNetworkPtr network, virNetworkDHCPLeasePtr *leases, unsigned int nleases); What struct contents do you propose for virNetworkDHCPLeasePtr? Are we expressing the output in stringized or raw form? That is, would a MAC address represented in that struct take exactly 6 bytes, even if some of them are the NUL byte, or would it be the stringized 18-byte NUL-terminated string? And/or this virNetworkGetDHCPLeaseForMAC(virNetworkPtr network, unsigned char *macaddr, I personally think the public API should stick to stringized representations. Yes, it's less friendly to machine code, but internally, our src/util/virmacaddr.h helps transfer between stringized and binary. Furthermore, we already have other API that operates on stringized versions, but none that operates on raw (see virDomainSetInterfaceParameters). Knowing what representation we are targetting impacts how this API will look. -- 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] RFC: Introduce API to return configuration/state paths of the network driver
On 08/13/2013 04:48 PM, Eric Blake wrote: virNetworkGetDHCPLeaseForMAC(virNetworkPtr network, unsigned char *macaddr, I personally think the public API should stick to stringized representations. Yes, it's less friendly to machine code, but internally, our src/util/virmacaddr.h helps transfer between stringized and binary. Furthermore, we already have other API that operates on stringized versions, but none that operates on raw (see virDomainSetInterfaceParameters). Knowing what representation we are targetting impacts how this API will look. For comparison, look at the API for virDomainLookupByUUID (takes a raw uuid - fixed number of bytes, unsigned char* argument) vs. virDomainLookupByUUIDString (takes a stringized uuid, char* argument). If efficiency were a concern, then I'd propose that we have two API: virNetworkGetDHCPLeaseForMAC(virNetworkPtr network, unsigned char *macaddr, ...) virNetworkGetDHCPLeaseForMACString(virNetworkPtr network, char *macaddr, ...) But I don't think efficiency is a concern, and rather than add a new API that has to have dual forms, I'd rather stick with the shorter name but using the stringized form of MAC. -- 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] [PATCH] network: permit upstream forwarding of unqualified DNS names
This resolves the issue that prompted the filing of https://bugzilla.redhat.com/show_bug.cgi?id=928638 (although the request there is for something much larger and more general than this patch). commit f3868259ca0517212e439a65c9060868f673b6c9 disabled the forwarding to upstream DNS servers of unresolved DNS requests for names that had no domain, but were just simple host names (no . character anywhere in the name). While this behavior is frowned upon by DNS root servers (that's why it was changed in libvirt), it is convenient in some cases, and since dnsmasq can be configured to allow it, it must not be strictly forbidden. This patch restores the old behavior, but since it is usually undesirable, restoring it requires specification of a new option in the network config. Adding the attribute forwardPlainNames='yes' to the dns elemnt does the trick - when that attribute is added to a network config, any simple hostnames that can't be resolved by the network's dnsmasq instance will be forwarded to the DNS servers listed in the host's /etc/resolv.conf for an attempt at resolution (just as any FQDN would be forwarded). When that attribute *isn't* specified, unresolved simple names will *not* be forwarded to the upstream DNS server - this is the default behavior. --- docs/formatnetwork.html.in | 25 +++ docs/schemas/network.rng | 8 +++ src/conf/network_conf.c| 28 -- src/conf/network_conf.h| 1 + src/network/bridge_driver.c| 20 +++- .../nat-network-dns-forward-plain.conf | 11 + .../nat-network-dns-forward-plain.xml | 9 +++ tests/networkxml2conftest.c| 1 + .../nat-network-dns-forward-plain.xml | 9 +++ .../nat-network-dns-forward-plain.xml | 11 + tests/networkxml2xmltest.c | 1 + 11 files changed, 112 insertions(+), 12 deletions(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-forward-plain.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-forward-plain.xml create mode 100644 tests/networkxml2xmlin/nat-network-dns-forward-plain.xml create mode 100644 tests/networkxml2xmlout/nat-network-dns-forward-plain.xml diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index eb7c4c7..e1482db 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -663,10 +663,27 @@ with the idiosyncrasies of the platform where libvirt is running. span class=sinceSince 0.8.8/span /dd - dtcodedns/code/dtdd -The dns element of a network contains configuration information for the -virtual network's DNS server. span class=sinceSince 0.9.3/span -Currently supported elements are: + dtcodedns/code/dt + dd The dns element of a network contains configuration +information for the virtual network's DNS +server span class=sinceSince 0.9.3/span. + +p + The dns element + can have an optional codeforwardPlainNames/code + attribute span class=sinceSince 1.1.2/span. + If codeforwardPlainNames/code is no, then DNS resolution + requests for names that are not qualified with a domain + (i.e. names with no . character) will not be forwarded to + the host's upstream DNS server - they will only be resolved if + they are known locally within the virtual network's own DNS + server. If codeforwardPlainNames/code is yes, + unqualified names bwill/b be forwarded to the upstream DNS + server if they can't be resolved by the virtual network's own + DNS server. +/p + +Currently supported sub-elements of codelt;dnsgt;/code are: dl dtcodetxt/code/dt ddA codedns/code element can have 0 or more codetxt/code elements. diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index ded8580..ab183f1 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -208,6 +208,14 @@ and other features in the dns element -- optional element name=dns + optional +attribute name=forwardPlainNames + choice +valueyes/value +valueno/value + /choice +/attribute + /optional zeroOrMore element name=txt attribute name=nameref name=dnsName//attribute diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 9141bbb..bbc980b 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1037,6 +1037,7 @@ virNetworkDNSDefParseXML(const char *networkName, xmlNodePtr *hostNodes = NULL; xmlNodePtr *srvNodes =
Re: [libvirt] RFC: Introduce API to return configuration/state paths of the network driver
On Wed, Aug 14, 2013 at 4:29 AM, Eric Blake ebl...@redhat.com wrote:
On 08/13/2013 04:48 PM, Eric Blake wrote:
virNetworkGetDHCPLeaseForMAC(virNetworkPtr network,
unsigned char *macaddr,
I personally think the public API should stick to stringized
representations. Yes, it's less friendly to machine code, but
internally, our src/util/virmacaddr.h helps transfer between stringized
and binary. Furthermore, we already have other API that operates on
stringized versions, but none that operates on raw (see
virDomainSetInterfaceParameters). Knowing what representation we are
targetting impacts how this API will look.
For comparison, look at the API for virDomainLookupByUUID (takes a raw
uuid - fixed number of bytes, unsigned char* argument) vs.
virDomainLookupByUUIDString (takes a stringized uuid, char* argument).
If efficiency were a concern, then I'd propose that we have two API:
virNetworkGetDHCPLeaseForMAC(virNetworkPtr network,
unsigned char *macaddr, ...)
virNetworkGetDHCPLeaseForMACString(virNetworkPtr network,
char *macaddr, ...)
But I don't think efficiency is a concern, and rather than add a new API
that has to have dual forms, I'd rather stick with the shorter name but
using the stringized form of MAC.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
I would like to see the API as:
/**
* virNetworkGetDHCPLeases:
* @network: pointer to network object
* @macaddr: MAC Address of an interface
* @leases: pointer to an array of structs which will hold the leases
* @nleases: number of leases in output
* @flags: extra flags, not used yet, so callers should always pass 0
*
* The API returns the leases of all interfaces by default, and if
* @macaddr is specified,.only the lease of the interface which
* matches the @macaddr is returned.
*
* Returns number of leases on success, -1 otherwise
*/
int virNetworkGetDHCPLeases(virNetworkPtr network,
const char *macaddr,
virNetworkDHCPLeasesPtr *leases,
int *nleases,
unsigned int flags);
Structs to be used:
/*In include/libvirt/libvirt.h.in */
struct _virNetworkDHCPLeases {
long long expirytime;
char *macaddr;
char *ipaddr;
char *hostname;
char *clientid;
};
/*In src/remote/remote_protocol.x*/
struct remote_network_dhcp_leases {
hyper expirytime;
remote_nonnull_string macaddr;
remote_nonnull_string ipaddr;
remote_nonnull_string hostname;
remote_nonnull_string clientid;
};
struct remote_network_get_dhcp_leases_args {
remote_nonnull_network net;
remote_string macaddr;
unsigned int flags;
};
struct remote_network_get_dhcp_leases_ret {
remote_network_dhcp_leases leases;
};
The following two blocks are required more than one, so should
we be introducing helper functions for them?
if ((VIR_STRDUP((*leases)[0].macaddr, leaseparams[1]) 0) ||
(VIR_STRDUP((*leases)[0].ipaddr, leaseparams[2]) 0) ||
(VIR_STRDUP((*leases)[0].hostname, leaseparams[3]) 0) ||
(VIR_STRDUP((*leases)[0].clientid, leaseparams[4]) 0))
goto cleanup;
for (i = 0; i nleases; i++) {
VIR_FREE(leases[i].macaddr);
VIR_FREE(leases[i].ipaddr);
VIR_FREE(leases[i].hostname);
VIR_FREE(leases[i].clientid);
}
--
Nehal J. Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Address missed feedback from review of virt-login-shell
On 08/13/2013 12:09 PM, Ruben Kerkhof wrote:
On Tue, Aug 13, 2013 at 1:16 PM, Daniel P. Berrange
berra...@redhat.comwrote:
virReportSystemError(errno, _(Unable exec shell %s), shargv[0]);
s/Unable/Unable to/
Pushed the fix in your name, along with another line with the same
problem (unable chdir(%s), and wrapping some long lines:
From 11cdc424d30b15c6780d546a2f0d8ff93ce291b6 Mon Sep 17 00:00:00 2001
From: Ruben Kerkhof ru...@rubenkerkhof.com
Date: Tue, 13 Aug 2013 17:28:06 -0600
Subject: [PATCH] virt-login-shell: improve error message grammar
and wrap some long lines
Signed-off-by: Eric Blake ebl...@redhat.com
---
tools/virt-login-shell.c | 18 --
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index 1157cd0..c754ae4 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -76,7 +76,8 @@ static int virLoginShellAllowedUser(virConfPtr conf,
/* Calc length and check items */
for (pp = p-list; pp; pp = pp-next) {
if (pp-type != VIR_CONF_STRING) {
-virReportSystemError(EINVAL, %s, _(shell must be a
list of strings));
+virReportSystemError(EINVAL, %s,
+ _(shell must be a list of strings));
goto cleanup;
} else {
/*
@@ -105,7 +106,9 @@ static int virLoginShellAllowedUser(virConfPtr conf,
}
}
}
-virReportSystemError(EPERM, _(%s not matched against
'allowed_users' in %s), name, conf_file);
+virReportSystemError(EPERM,
+ _(%s not matched against 'allowed_users' in %s),
+ name, conf_file);
cleanup:
VIR_FREE(gname);
return ret;
@@ -128,7 +131,8 @@ static char **virLoginShellGetShellArgv(virConfPtr conf)
/* Calc length and check items */
for (len = 0, pp = p-list; pp; len++, pp = pp-next) {
if (pp-type != VIR_CONF_STRING) {
-virReportSystemError(EINVAL, %s, _(shell must be a
list of strings));
+virReportSystemError(EINVAL, %s,
+ _(shell must be a list of strings));
goto error;
}
}
@@ -248,7 +252,8 @@ main(int argc, char **argv)
}
if (uid == 0) {
-virReportSystemError(EPERM, _(%s must be run by non root
users), progname);
+virReportSystemError(EPERM, _(%s must be run by non root users),
+ progname);
goto cleanup;
}
@@ -340,11 +345,12 @@ main(int argc, char **argv)
if (ccpid == 0) {
if (chdir(homedir) 0) {
-virReportSystemError(errno, _(Unable chdir(%s)),
homedir);
+virReportSystemError(errno, _(Unable to chdir(%s)),
homedir);
return EXIT_FAILURE;
}
if (execv(shargv[0], (char *const*) shargv) 0) {
-virReportSystemError(errno, _(Unable exec shell %s),
shargv[0]);
+virReportSystemError(errno, _(Unable to exec shell %s),
+ shargv[0]);
return EXIT_FAILURE;
}
}
--
1.7.1
--
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 v2]LXC: Helper function for checking ownership of dir when userns enabled
-Original Message-
From: Daniel P. Berrange [mailto:berra...@redhat.com]
Sent: Saturday, August 10, 2013 12:54 AM
To: Chen Hanxiao
Cc: libvir-list@redhat.com
Subject: Re: [libvirt] [PATCH v2]LXC: Helper function for checking ownership
of
dir when userns enabled
On Fri, Aug 09, 2013 at 04:05:58PM +0800, Chen Hanxiao wrote:
From: Chen Hanxiao chenhanx...@cn.fujitsu.com
If we enable userns, the ownership of dir we provided for containers
should match the uid/gid in idmap.
Currently, the debug log is very implicit or misleading sometimes.
This patch will help clarify this for us when using
debug log or virsh.
I do recall hitting some permission issue once, but can't remember
just what it was. Can you describe exactly how to reproduce the
problem ?
1) Enable user namespace in kernel
2) Add idmap for container
3) Don't change the ownership of devices/ filesystem/ source dir ( leave them
to 'root' for instance)
4) Start the container
Usually I got an input/output error by virsh, which is not a good hint.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/lxc/lxc_container.c | 46
++
1 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b910b10..2ccdc61 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1815,6 +1815,49 @@ lxcNeedNetworkNamespace(virDomainDefPtr
def)
return false;
}
+/*
+ * Helper function for helping check
+ * whether we have enough privilege
+ * to operate the source dir when userns enabled
+ * @vmDef: pointer to vm definition structure
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
+{
+struct stat buf;
+size_t i;
+uid_t uid;
+gid_t gid;
+
+VIR_DEBUG(vmDef-nfss %d, (int)vmDef-nfss);
+for (i = 0; i vmDef-nfss; i++) {
+VIR_DEBUG(dst is %s, src is %s,
+ vmDef-fss[i]-dst,
+ vmDef-fss[i]-src);
+
+uid = vmDef-idmap.uidmap[0].target;
+gid = vmDef-idmap.gidmap[0].target;
+
+if (lstat(vmDef-fss[i]-src, buf) 0) {
+virReportSystemError(errno, _(Cannot access '%s'),
+ vmDef-fss[i]-src);
+return -1;
+} else if (uid != buf.st_uid || gid != buf.st_gid) {
+VIR_DEBUG(In userns uid is %d, gid is %d\n,
+ uid, gid);
+errno = EINVAL;
+
+virReportSystemError(errno,
+ _([userns] Src dir '%s' does not
belong to uid/gid: %d/%d),
+ vmDef-fss[i]-src, uid, gid);
+return -1;
+}
+}
+
+return 0;
+}
+
/**
* lxcContainerStart:
* @def: pointer to virtual machine structure
@@ -1866,6 +1909,9 @@ int lxcContainerStart(virDomainDefPtr def,
if (userns_supported()) {
VIR_DEBUG(Enable user namespace);
cflags |= CLONE_NEWUSER;
+if (lxcContainerUsernsSrcOwnershipCheck(def) 0) {
+return -1;
+}
} else {
virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED,
%s,
_(Kernel doesn't support user
namespace));
Daniel
--
|: http://berrange.com -o-
http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o-
http://virt-manager.org :|
|: http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o-
http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
