[libvirt] [PATCHv2] Don't log an internal error when the guest hasn't updated balloon stats
If virDomainMemoryStats is called too soon after domain startup,
QEMU returns:
error:{class:GenericError,desc:guest hasn't updated any stats yet}
when we try to query balloon stats.
Check for this reply and log it as OPERATION_INVALID instead of
INTERNAL_ERROR. This means the daemon only logs it at the debug level,
without polluting system logs.
Reported by Laszlo Pal:
https://www.redhat.com/archives/libvirt-users/2014-May/msg00023.html
---
v1: https://www.redhat.com/archives/libvir-list/2014-May/msg00420.html
v2:
return 0 in this case - even though balloon stats are not yet available,
we can still return 'rss' in qemuDomainMemoryStats
jump to cleanup if CheckError returns 0
src/qemu/qemu_monitor_json.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f8ab975..914f3ef 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1465,12 +1465,22 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
NULL)))
goto cleanup;
-ret = qemuMonitorJSONCommand(mon, cmd, reply);
+if ((ret = qemuMonitorJSONCommand(mon, cmd, reply)) 0)
+goto cleanup;
-if (ret == 0)
-ret = qemuMonitorJSONCheckError(cmd, reply);
+if ((data = virJSONValueObjectGet(reply, error))) {
+const char *klass = virJSONValueObjectGetString(data, class);
+const char *desc = virJSONValueObjectGetString(data, desc);
-if (ret 0)
+if (STREQ_NULLABLE(klass, GenericError)
+STREQ_NULLABLE(desc, guest hasn't updated any stats yet)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(the guest hasn't updated any stats yet));
+goto cleanup;
+}
+}
+
+if ((ret = qemuMonitorJSONCheckError(cmd, reply)) 0)
goto cleanup;
if (!(data = virJSONValueObjectGet(reply, return))) {
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3] Add invariant TSC cpu flag
Add suport for invariant TSC flag (CPUID 0x8007, bit 8 of EDX).
If this flag is enabled, the TSC ticks at a constant rate across
all ACPI P-, C- and T-states.
This can be enabled by adding:
feature name='invtsc'/
to the cpu element.
Migration and saving the domain does not work with this flag.
QEMU support for this is not merged yet:
https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
The feature name invtsc differs from the name used by the linux kernel:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/powerflags.c?id=30321c7b#n18
---
v1: https://www.redhat.com/archives/libvir-list/2014-May/msg00183.html
v2: https://www.redhat.com/archives/libvir-list/2014-May/msg00297.html
add it as a cpu flag instead of a timer
v3:
check if the feature wasn't filtered out on domain startup
src/cpu/cpu_map.xml | 5 +
src/qemu/qemu_migration.c | 14 ++
src/qemu/qemu_process.c | 15 +++
3 files changed, 34 insertions(+)
diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 7d34d40..ffaeb92 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -327,6 +327,11 @@
cpuid function='0x0007' ebx='0x0010'/
/feature
+!-- Advanced Power Management edx features --
+feature name='invtsc'
+ cpuid function='0x8007' edx='0x0100'/
+/feature
+
!-- models --
model name='486'
feature name='fpu'/
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index f0df1a6..7504a38 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1513,6 +1513,20 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver,
virDomainObjPtr vm,
return false;
}
+for (i = 0; i def-cpu-nfeatures; i++) {
+virCPUFeatureDefPtr feature = def-cpu-features[i];
+
+if (feature-policy != VIR_CPU_FEATURE_REQUIRE)
+continue;
+
+if (STREQ(feature-name, invtsc)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(domain has CPU feature: %s),
+ feature-name);
+return false;
+}
+}
+
return true;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a83780f..0824afe 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3568,6 +3568,7 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
virDomainObjPtr vm)
qemuDomainObjPrivatePtr priv = vm-privateData;
int rc;
bool ret = false;
+size_t i;
switch (arch) {
case VIR_ARCH_I686:
@@ -3590,6 +3591,20 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
virDomainObjPtr vm)
goto cleanup;
}
}
+
+for (i = 0; i def-cpu-nfeatures; i++) {
+virCPUFeatureDefPtr feature = def-cpu-features[i];
+
+if (feature-policy != VIR_CPU_FEATURE_REQUIRE)
+continue;
+
+if (STREQ(feature-name, invtsc)
+!cpuHasFeature(guestcpu, feature-name)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(host doesn't support invariant TSC));
+goto cleanup;
+}
+}
break;
default:
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [patch v3 2/2] add inotify handler to qemu driver
On Wed, May 07, 2014 at 18:11:50 +0800, Chen Fan wrote: we don't expect to reload 'migrate_uri' with restarting libvirtd everytime while updating the URI, so adding inotify handler to reload 'migrate_uri' configuration without restarting libvirtd, it will be also helpful for virt-manager to get 'migrate_uri'. NACK, if we ever want configuration to be automatically reloaded when configuration file changes (which I seriously doubt, SIGHUP is the standard way of reloading configuration files), we certainly would not want to do it this hacky way and only for one specific option. Not to mention that the content of virQEMUDriverConfig must not change, a completely new structure has to be created with the changed values. Anyway, it's good you removed it from v4. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/1] migration: add support for migrateURI configuration
On Wed, May 14, 2014 at 15:18:09 +0800, Chen Fan wrote:
For now, we set the migration URI via command line '--migrate_uri' or
construct the URI by looking up the dest host's hostname which could be
solved by DNS automatically.
But in cases the dest host have two or more NICs to reach, we may need to
send the migration data over a specific NIC which is different from the
automatically resloved one for some reason like performance, security, etc.
thus we must explicitly specify the migrateuri in command line everytime,
but it is too troublesome if there are many such hosts(and don't forget
virt-manager).
This patches add a configuration file option on dest host to save the
default migrate uri which explicitly specify which of this host's
addresses is used for transferring data, thus user doesn't boring
to specify it in command line everytime.
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
---
v3-v4: move up the default uri_in setting to
qemuDomainMigratePrepare3Params()
src/qemu/qemu.conf | 6 +-
src/qemu/qemu_conf.c | 1 +
src/qemu/qemu_conf.h | 1 +
src/qemu/qemu_driver.c | 2 +-
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index f0e802f..6b443d0 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -449,7 +449,11 @@
#
#seccomp_sandbox = 1
-
+# Override the migration URI for specifying one of host's IP addresses
+# to transfer the migration data stream.
+# Defaults to hostname, both IPv4 and IPv6 addresses are accepted.
+#
+#migrate_uri = tcp://192.168.0.1
# Override the listen address for all incoming migrations. Defaults to
# 0.0.0.0, or :: if both host and qemu are capable of IPv6.
The more I think about this the more I incline to a slightly different
approach. Rather than providing a way to override migration URI, we
could just provide an option in libvirtd.conf to override what
virGetHostname returns. That is, the option (naturally called hostname)
would tell libvirt to use the configured hostname (which might even be
just an IP address) instead of trying to detect it.
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 198ee2f..43361dc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -574,6 +574,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr
cfg,
GET_VALUE_LONG(seccomp_sandbox, cfg-seccompSandbox);
+GET_VALUE_STR(migrate_uri, cfg-migrateUri);
GET_VALUE_STR(migration_address, cfg-migrationAddress);
ret = 0;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a36ea63..f99c56e 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -163,6 +163,7 @@ struct _virQEMUDriverConfig {
int seccompSandbox;
+char *migrateUri;
/* The default for -incoming */
char *migrationAddress;
int migrationPortMin;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fca1a91..56c24b5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10888,7 +10888,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
virDomainDefPtr def = NULL;
const char *dom_xml = NULL;
const char *dname = NULL;
-const char *uri_in = NULL;
+const char *uri_in = cfg-migrateUri;
const char *listenAddress = cfg-migrationAddress;
char *origname = NULL;
int ret = -1;
And in any case, the change you made between v3 and v4 is wrong, since
now you are only change one entry point to the Prepare phase while
changing qemuMigrationPrepareDirect makes this work for all APIs and
migration protocol versions.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: For virsh migrate --tunnelled, use the virDomainMigrateToURI3 API.
On Thu, May 08, 2014 at 21:49:40 +0100, Richard W.M. Jones wrote:
Using virsh migrate + the --tunnelled flag causes virsh to use the
wrong API. This gives the error:
error: use virDomainMigrateToURI3 for peer-to-peer migration
As the error message is wrong, this patch also corrects the error
message.
https://bugzilla.redhat.com/show_bug.cgi?id=1095924
Right, the error message is wrong but the suggested doesn't seem right
either. The error you should see for virsh migrate --tunnelled is
cannot perform tunnelled migration without using peer2peer flag. Virsh
should not try to guess that tunnelled implies peer2peer because there's
a possibility someone implements tunnelled migration that transfers data
through the client which controls migration and thus p2p flag would not
be required and virDomainMigrateToURI3 would be a wrong API. Not that
the APIs are designed for that but if I correctly remember discussions
we had about this topic in the past, the goal was virsh should not be
enforcing (or even automatically adding) p2p flag for tunnelled
migrations for better compatibility with future.
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
Cc: Jiri Denemark jdene...@redhat.com
---
src/libvirt.c| 2 +-
tools/virsh-domain.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 2cd793c..6a361f6 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5726,7 +5726,7 @@ virDomainMigrate3(virDomainPtr domain,
if (flags (VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_TUNNELLED)) {
virReportInvalidArg(flags, %s,
_(use virDomainMigrateToURI3 for peer-to-peer
- migration));
+ or tunnelled migration));
goto error;
}
In other words, I think we actually want VIR_MIGRATE_TUNNELLED to be
removed from the condition above and just add another condition similar
to what we have in the other migration APIs:
if (flags VIR_MIGRATE_TUNNELLED) {
virReportInvalidArg(flags, %s,
_(cannot perform tunnelled migration
without using peer2peer flag));
goto error;
}
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu_driver: clamp value for setting sched cpu_shares with --config.
As shown in 'man virsh' about schedinfo:
Note: The cpu_shares parameter has a valid value range of 0-262144;
Negative values are wrapped to positive, and larger values are
capped at
the maximum. Therefore, -1 is a useful shorthand for 262144.
On the Linux kernel, the values 0 and 1 are automatically converted
to
a minimal value of 2.
it works well with --live, but not with --config.
Example:
# virsh schedinfo rhel7-ful --set cpu_shares=0 --config
Scheduler : posix
cpu_shares : 0
vcpu_period: 0
vcpu_quota : 0
emulator_period: 0
emulator_quota : 0
cpu_shares is 0 rather than expected 2.
What's worse, when we start it again, it is the default value of
cpu_shares 1024.
Because when we set the value of cpu_shares, when flag is --live,
write the value into cgroup/cpu.shares. Then it will convert the
value into the range of [2, 262144]. When flag is --config, we
set the value into vmdef immidiately and 0 means no settting for
cpu_shares. When we start vm again, libvirt use default value(1024)
for it.
This patch clamp the cpu_shares value when flag is --config, then
we will get then correct settting in output of virsh schedinfo
and value in cgroup after next booting of vm.
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/qemu/qemu_driver.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 52ca47c..7648865 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -112,6 +112,8 @@ VIR_LOG_INIT(qemu.qemu_driver);
#define QEMU_SCHED_MAX_PERIOD 100LL
#define QEMU_SCHED_MIN_QUOTA 1000LL
#define QEMU_SCHED_MAX_QUOTA 18446744073709551LL
+#define QEMU_SCHED_MIN_SHARES 2LL
+#define QEMU_SCHED_MAX_SHARES262144LL
#if HAVE_LINUX_KVM_H
# include linux/kvm.h
@@ -9063,7 +9065,9 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
}
if (flags VIR_DOMAIN_AFFECT_CONFIG) {
-vmdef-cputune.shares = value_ul;
+vmdef-cputune.shares = CLAMP(value_ul,
+ QEMU_SCHED_MIN_SHARES,
+ QEMU_SCHED_MAX_SHARES);
vmdef-cputune.sharesSpecified = true;
}
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] {qemu|test}_driver: Use CLAMP macro to clamp value.
As we have CLAMP macro in util.h, we should use it to replace
the open implementations of it.
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/qemu/qemu_driver.c | 8 ++--
src/test/test_driver.c | 7 ++-
2 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7648865..96f325d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4547,9 +4547,7 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
if (maxcpu hostcpus)
maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (ncpumaps targetDef-vcpus)
-ncpumaps = targetDef-vcpus;
+ncpumaps = CLAMP(ncpumaps, ncpumaps, targetDef-vcpus);
if (ncpumaps 1) {
goto cleanup;
@@ -4857,9 +4855,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
if (maxcpu hostcpus)
maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (maxinfo priv-nvcpupids)
-maxinfo = priv-nvcpupids;
+maxinfo = CLAMP(maxinfo, maxinfo, priv-nvcpupids);
if (maxinfo = 1) {
if (info != NULL) {
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 37756e7..024f63d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2773,12 +2773,9 @@ static int testDomainGetVcpus(virDomainPtr domain,
hostcpus = VIR_NODEINFO_MAXCPUS(privconn-nodeInfo);
maxcpu = maplen * 8;
-if (maxcpu hostcpus)
-maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (maxinfo privdom-def-vcpus)
-maxinfo = privdom-def-vcpus;
+maxcpu = CLAMP(maxcpu, maxcpu, hostcpus);
+maxinfo = CLAMP(maxinfo, maxinfo, privdom-def-vcpus);
/* Populate virVcpuInfo structures */
if (info != NULL) {
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] virutil: Introduce a new macro named CLAMP.
This patch introduce a new macro to return a
value clamped to a given range.
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/util/virutil.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 2bb74e2..e8536d8 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -37,6 +37,12 @@
# ifndef MAX
# define MAX(a, b) ((a) (b) ? (a) : (b))
# endif
+# ifndef CLAMP
+# define CLAMP(v, min, max) ({ \
+typeof(v) _v = v; \
+_v = _v min ? min: _v;\
+_v max ? max: _v; })
+# endif
int virSetBlocking(int fd, bool blocking) ATTRIBUTE_RETURN_CHECK;
int virSetNonBlock(int fd) ATTRIBUTE_RETURN_CHECK;
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: For virsh migrate --tunnelled, use the virDomainMigrateToURI3 API.
On Thu, May 15, 2014 at 12:30:24PM +0200, Jiri Denemark wrote:
On Thu, May 08, 2014 at 21:49:40 +0100, Richard W.M. Jones wrote:
Using virsh migrate + the --tunnelled flag causes virsh to use the
wrong API. This gives the error:
error: use virDomainMigrateToURI3 for peer-to-peer migration
As the error message is wrong, this patch also corrects the error
message.
https://bugzilla.redhat.com/show_bug.cgi?id=1095924
Right, the error message is wrong but the suggested doesn't seem right
either. The error you should see for virsh migrate --tunnelled is
cannot perform tunnelled migration without using peer2peer flag. Virsh
should not try to guess that tunnelled implies peer2peer because there's
a possibility someone implements tunnelled migration that transfers data
through the client which controls migration and thus p2p flag would not
be required and virDomainMigrateToURI3 would be a wrong API. Not that
the APIs are designed for that but if I correctly remember discussions
we had about this topic in the past, the goal was virsh should not be
enforcing (or even automatically adding) p2p flag for tunnelled
migrations for better compatibility with future.
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
Cc: Jiri Denemark jdene...@redhat.com
---
src/libvirt.c| 2 +-
tools/virsh-domain.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 2cd793c..6a361f6 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5726,7 +5726,7 @@ virDomainMigrate3(virDomainPtr domain,
if (flags (VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_TUNNELLED)) {
virReportInvalidArg(flags, %s,
_(use virDomainMigrateToURI3 for peer-to-peer
- migration));
+ or tunnelled migration));
goto error;
}
In other words, I think we actually want VIR_MIGRATE_TUNNELLED to be
removed from the condition above and just add another condition similar
to what we have in the other migration APIs:
if (flags VIR_MIGRATE_TUNNELLED) {
virReportInvalidArg(flags, %s,
_(cannot perform tunnelled migration
without using peer2peer flag));
goto error;
}
I'll defer to whatever you want to do. When I originally submitted
the patch, I believe that tunnelled !p2p migration was possible, based
on this diagram:
http://libvirt.org/migration.html#flowmanageddirect
I think the diagram and error message are both misleading.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Avoid leak in qemuDomainCheckRemoveOptionalDisk
Coverity complains about event being leaked in
qemuDomainCheckRemoveOptionalDisk. The best fix for it is to avoid
searching for the disk (using its srouce path even) we got by walking
through vm-def-disks array.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_domain.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7a0be12..78cfdc6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2176,11 +2176,11 @@ qemuDomainSetFakeReboot(virQEMUDriverPtr driver,
static int
qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- virDomainDiskDefPtr disk)
+ size_t diskIndex)
{
char uuid[VIR_UUID_STRING_BUFLEN];
virObjectEventPtr event = NULL;
-virDomainDiskDefPtr del_disk = NULL;
+virDomainDiskDefPtr disk = vm-def-disks[diskIndex];
const char *src = virDomainDiskGetSource(disk);
virUUIDFormat(vm-def-uuid, uuid);
@@ -2200,13 +2200,8 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr
driver,
event = virDomainEventDiskChangeNewFromObj(vm, src, NULL,
disk-info.alias,
VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START);
-
-if (!(del_disk = virDomainDiskRemoveByName(vm-def, src))) {
-virReportError(VIR_ERR_INVALID_ARG,
- _(no source device %s), src);
-return -1;
-}
-virDomainDiskDefFree(del_disk);
+virDomainDiskRemove(vm-def, diskIndex);
+virDomainDiskDefFree(disk);
}
if (event)
@@ -2218,11 +2213,11 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr
driver,
static int
qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- virDomainDiskDefPtr disk,
+ size_t diskIndex,
bool cold_boot)
{
char uuid[VIR_UUID_STRING_BUFLEN];
-int startupPolicy = disk-startupPolicy;
+int startupPolicy = vm-def-disks[diskIndex]-startupPolicy;
virUUIDFormat(vm-def-uuid, uuid);
@@ -2244,7 +2239,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
break;
}
-if (qemuDomainCheckRemoveOptionalDisk(driver, vm, disk) 0)
+if (qemuDomainCheckRemoveOptionalDisk(driver, vm, diskIndex) 0)
goto error;
return 0;
@@ -2282,11 +2277,11 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
{
int ret = -1;
size_t i;
-virDomainDiskDefPtr disk;
VIR_DEBUG(Checking for disk presence);
for (i = vm-def-ndisks; i 0; i--) {
-disk = vm-def-disks[i - 1];
+size_t idx = i - 1;
+virDomainDiskDefPtr disk = vm-def-disks[idx];
const char *path = virDomainDiskGetSource(disk);
virStorageFileFormat format = virDomainDiskGetFormat(disk);
virStorageType type = virStorageSourceGetActualType(disk-src);
@@ -2308,7 +2303,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
continue;
if (disk-startupPolicy
-qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
+qemuDomainCheckDiskStartupPolicy(driver, vm, idx,
cold_boot) = 0) {
virResetLastError();
continue;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] {qemu|test}_driver: Use CLAMP macro to clamp value.
On Thu, May 15, 2014 at 06:39:51PM +0900, Dongsheng Yang wrote:
As we have CLAMP macro in util.h, we should use it to replace
the open implementations of it.
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/qemu/qemu_driver.c | 8 ++--
src/test/test_driver.c | 7 ++-
2 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7648865..96f325d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4547,9 +4547,7 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
if (maxcpu hostcpus)
maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (ncpumaps targetDef-vcpus)
-ncpumaps = targetDef-vcpus;
+ncpumaps = CLAMP(ncpumaps, ncpumaps, targetDef-vcpus);
Useless comparison to self; I'd suggest using MIN() instead. This
applies to the patch.
Martin
if (ncpumaps 1) {
goto cleanup;
@@ -4857,9 +4855,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
if (maxcpu hostcpus)
maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (maxinfo priv-nvcpupids)
-maxinfo = priv-nvcpupids;
+maxinfo = CLAMP(maxinfo, maxinfo, priv-nvcpupids);
if (maxinfo = 1) {
if (info != NULL) {
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 37756e7..024f63d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2773,12 +2773,9 @@ static int testDomainGetVcpus(virDomainPtr domain,
hostcpus = VIR_NODEINFO_MAXCPUS(privconn-nodeInfo);
maxcpu = maplen * 8;
-if (maxcpu hostcpus)
-maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (maxinfo privdom-def-vcpus)
-maxinfo = privdom-def-vcpus;
+maxcpu = CLAMP(maxcpu, maxcpu, hostcpus);
+maxinfo = CLAMP(maxinfo, maxinfo, privdom-def-vcpus);
/* Populate virVcpuInfo structures */
if (info != NULL) {
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] {qemu|test}_driver: Use CLAMP macro to clamp value.
On 05/15/2014 08:37 PM, Martin Kletzander wrote:
On Thu, May 15, 2014 at 06:39:51PM +0900, Dongsheng Yang wrote:
As we have CLAMP macro in util.h, we should use it to replace
the open implementations of it.
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/qemu/qemu_driver.c | 8 ++--
src/test/test_driver.c | 7 ++-
2 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7648865..96f325d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4547,9 +4547,7 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
if (maxcpu hostcpus)
maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (ncpumaps targetDef-vcpus)
-ncpumaps = targetDef-vcpus;
+ncpumaps = CLAMP(ncpumaps, ncpumaps, targetDef-vcpus);
Useless comparison to self; I'd suggest using MIN() instead. This
applies to the patch.
Oops, did not realize that. Just found the key word of Clamp in
comment and
replaced it with CLAMP.
So it is not related with the CLAMP, and I will sent a v2 of this patch
independently.
Thanx.
Martin
if (ncpumaps 1) {
goto cleanup;
@@ -4857,9 +4855,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
if (maxcpu hostcpus)
maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (maxinfo priv-nvcpupids)
-maxinfo = priv-nvcpupids;
+maxinfo = CLAMP(maxinfo, maxinfo, priv-nvcpupids);
if (maxinfo = 1) {
if (info != NULL) {
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 37756e7..024f63d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2773,12 +2773,9 @@ static int testDomainGetVcpus(virDomainPtr
domain,
hostcpus = VIR_NODEINFO_MAXCPUS(privconn-nodeInfo);
maxcpu = maplen * 8;
-if (maxcpu hostcpus)
-maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (maxinfo privdom-def-vcpus)
-maxinfo = privdom-def-vcpus;
+maxcpu = CLAMP(maxcpu, maxcpu, hostcpus);
+maxinfo = CLAMP(maxinfo, maxinfo, privdom-def-vcpus);
/* Populate virVcpuInfo structures */
if (info != NULL) {
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] virutil: Introduce a new macro named CLAMP.
On Thu, May 15, 2014 at 06:39:49PM +0900, Dongsheng Yang wrote:
This patch introduce a new macro to return a
value clamped to a given range.
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/util/virutil.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 2bb74e2..e8536d8 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -37,6 +37,12 @@
# ifndef MAX
# define MAX(a, b) ((a) (b) ? (a) : (b))
# endif
+# ifndef CLAMP
+# define CLAMP(v, min, max) ({ \
+typeof(v) _v = v; \
+_v = _v min ? min: _v;\
+_v max ? max: _v; })
+# endif
It's just my subjective impression, but wouldn't the following be a
bit more readable and less obfuscated?
#define CLAMP(v, min, max) MAX(MIN(v, max), min)
Martin.
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu_driver: clamp value for setting sched cpu_shares with --config.
On Thu, May 15, 2014 at 06:39:50PM +0900, Dongsheng Yang wrote:
As shown in 'man virsh' about schedinfo:
Note: The cpu_shares parameter has a valid value range of 0-262144;
Negative values are wrapped to positive, and larger values are
capped at
the maximum. Therefore, -1 is a useful shorthand for 262144.
On the Linux kernel, the values 0 and 1 are automatically converted
to
a minimal value of 2.
it works well with --live, but not with --config.
Example:
# virsh schedinfo rhel7-ful --set cpu_shares=0 --config
Scheduler : posix
cpu_shares : 0
vcpu_period: 0
vcpu_quota : 0
emulator_period: 0
emulator_quota : 0
cpu_shares is 0 rather than expected 2.
What's worse, when we start it again, it is the default value of
cpu_shares 1024.
Because when we set the value of cpu_shares, when flag is --live,
write the value into cgroup/cpu.shares. Then it will convert the
value into the range of [2, 262144]. When flag is --config, we
set the value into vmdef immidiately and 0 means no settting for
cpu_shares. When we start vm again, libvirt use default value(1024)
for it.
This patch clamp the cpu_shares value when flag is --config, then
we will get then correct settting in output of virsh schedinfo
and value in cgroup after next booting of vm.
I was under the impression that this was meant to be fixed by commit
97814d8a, what's the difference to that?
Martin
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/qemu/qemu_driver.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 52ca47c..7648865 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -112,6 +112,8 @@ VIR_LOG_INIT(qemu.qemu_driver);
#define QEMU_SCHED_MAX_PERIOD 100LL
#define QEMU_SCHED_MIN_QUOTA 1000LL
#define QEMU_SCHED_MAX_QUOTA 18446744073709551LL
+#define QEMU_SCHED_MIN_SHARES 2LL
+#define QEMU_SCHED_MAX_SHARES262144LL
#if HAVE_LINUX_KVM_H
# include linux/kvm.h
@@ -9063,7 +9065,9 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
}
if (flags VIR_DOMAIN_AFFECT_CONFIG) {
-vmdef-cputune.shares = value_ul;
+vmdef-cputune.shares = CLAMP(value_ul,
+ QEMU_SCHED_MIN_SHARES,
+ QEMU_SCHED_MAX_SHARES);
vmdef-cputune.sharesSpecified = true;
}
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu_driver: clamp value for setting sched cpu_shares with --config.
On 05/15/2014 08:56 PM, Martin Kletzander wrote:
On Thu, May 15, 2014 at 06:39:50PM +0900, Dongsheng Yang wrote:
As shown in 'man virsh' about schedinfo:
Note: The cpu_shares parameter has a valid value range of
0-262144;
Negative values are wrapped to positive, and larger values are
capped at
the maximum. Therefore, -1 is a useful shorthand for 262144.
On the Linux kernel, the values 0 and 1 are automatically
converted to
a minimal value of 2.
it works well with --live, but not with --config.
Example:
# virsh schedinfo rhel7-ful --set cpu_shares=0 --config
Scheduler : posix
cpu_shares : 0
vcpu_period: 0
vcpu_quota : 0
emulator_period: 0
emulator_quota : 0
cpu_shares is 0 rather than expected 2.
What's worse, when we start it again, it is the default value of
cpu_shares 1024.
Because when we set the value of cpu_shares, when flag is --live,
write the value into cgroup/cpu.shares. Then it will convert the
value into the range of [2, 262144]. When flag is --config, we
set the value into vmdef immidiately and 0 means no settting for
cpu_shares. When we start vm again, libvirt use default value(1024)
for it.
This patch clamp the cpu_shares value when flag is --config, then
we will get then correct settting in output of virsh schedinfo
and value in cgroup after next booting of vm.
I was under the impression that this was meant to be fixed by commit
97814d8a, what's the difference to that?
Commit 97814d8a is to make the output of virsh schedinfo --live showing
the real value in cgroup.
This patch here is to do another thing, it is about --config. When we
use --live
to set cpu_shares, as shown in man virsh, we depend on the conversion by
cgroup. But when we set it with --config, we did not actually write the
value
into cgroup, then the value will not be converted to the expected value
in manpage.
And as I described above, when we set cpu_shares=0 with --config, we
will get
1024 in next booting.
Martin
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/qemu/qemu_driver.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 52ca47c..7648865 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -112,6 +112,8 @@ VIR_LOG_INIT(qemu.qemu_driver);
#define QEMU_SCHED_MAX_PERIOD 100LL
#define QEMU_SCHED_MIN_QUOTA 1000LL
#define QEMU_SCHED_MAX_QUOTA 18446744073709551LL
+#define QEMU_SCHED_MIN_SHARES 2LL
+#define QEMU_SCHED_MAX_SHARES262144LL
#if HAVE_LINUX_KVM_H
# include linux/kvm.h
@@ -9063,7 +9065,9 @@
qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
}
if (flags VIR_DOMAIN_AFFECT_CONFIG) {
-vmdef-cputune.shares = value_ul;
+vmdef-cputune.shares = CLAMP(value_ul,
+ QEMU_SCHED_MIN_SHARES,
+ QEMU_SCHED_MAX_SHARES);
vmdef-cputune.sharesSpecified = true;
}
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] virutil: Introduce a new macro named CLAMP.
On 05/15/2014 08:53 PM, Martin Kletzander wrote:
On Thu, May 15, 2014 at 06:39:49PM +0900, Dongsheng Yang wrote:
This patch introduce a new macro to return a
value clamped to a given range.
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/util/virutil.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 2bb74e2..e8536d8 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -37,6 +37,12 @@
# ifndef MAX
# define MAX(a, b) ((a) (b) ? (a) : (b))
# endif
+# ifndef CLAMP
+# define CLAMP(v, min, max) ({ \
+typeof(v) _v = v; \
+_v = _v min ? min: _v;\
+_v max ? max: _v; })
+# endif
It's just my subjective impression, but wouldn't the following be a
bit more readable and less obfuscated?
#define CLAMP(v, min, max) MAX(MIN(v, max), min)
Neat! I think it works. I will use it in v2.
Thanx
Martin.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] cleanup: clamp max info with MIN().
Rather than using a open coded implementation,
this patch use MIN macro to clamp infomation
to allowed maxmum.
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/qemu/qemu_driver.c | 18 ++
src/test/test_driver.c | 10 +++---
2 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 52ca47c..b3782dd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4542,12 +4542,9 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
goto cleanup;
maxcpu = maplen * 8;
-if (maxcpu hostcpus)
-maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (ncpumaps targetDef-vcpus)
-ncpumaps = targetDef-vcpus;
+maxcpu = MIN(maxcpu, hostcpus);
+ncpumaps = MIN(ncpumaps, targetDef-vcpus);
if (ncpumaps 1) {
goto cleanup;
@@ -4786,8 +4783,8 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom,
goto cleanup;
maxcpu = maplen * 8;
-if (maxcpu hostcpus)
-maxcpu = hostcpus;
+
+maxcpu = MIN(maxcpu, hostcpus);
/* initialize cpumaps */
memset(cpumaps, 0xff, maplen);
@@ -4852,12 +4849,9 @@ qemuDomainGetVcpus(virDomainPtr dom,
goto cleanup;
maxcpu = maplen * 8;
-if (maxcpu hostcpus)
-maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (maxinfo priv-nvcpupids)
-maxinfo = priv-nvcpupids;
+maxcpu = MIN(maxcpu, hostcpus);
+maxinfo = MIN(maxinfo, priv-nvcpupids);
if (maxinfo = 1) {
if (info != NULL) {
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 37756e7..4e591f2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2773,12 +2773,9 @@ static int testDomainGetVcpus(virDomainPtr domain,
hostcpus = VIR_NODEINFO_MAXCPUS(privconn-nodeInfo);
maxcpu = maplen * 8;
-if (maxcpu hostcpus)
-maxcpu = hostcpus;
-/* Clamp to actual number of vcpus */
-if (maxinfo privdom-def-vcpus)
-maxinfo = privdom-def-vcpus;
+maxcpu = MIN(maxcpu, hostcpus);
+maxinfo = MIN(maxinfo, privdom-def-vcpus);
/* Populate virVcpuInfo structures */
if (info != NULL) {
@@ -2858,8 +2855,7 @@ static int testDomainPinVcpu(virDomainPtr domain,
privmaplen = VIR_CPU_MAPLEN(hostcpus);
maxcpu = maplen * 8;
-if (maxcpu hostcpus)
-maxcpu = hostcpus;
+maxcpu = MIN(maxcpu, hostcpus);
privcpumap = VIR_GET_CPUMAP(privdomdata-cpumaps, privmaplen, vcpu);
memset(privcpumap, 0, privmaplen);
--
1.8.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Avoid leak in qemuDomainCheckRemoveOptionalDisk
On 05/15/14 13:20, Jiri Denemark wrote: Coverity complains about event being leaked in qemuDomainCheckRemoveOptionalDisk. The best fix for it is to avoid searching for the disk (using its srouce path even) we got by walking through vm-def-disks array. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/qemu/qemu_domain.c | 25 ++--- 1 file changed, 10 insertions(+), 15 deletions(-) ACK, 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 2/3] qemu_driver: clamp value for setting sched cpu_shares with --config.
On 05/15/2014 11:39 AM, Dongsheng Yang wrote: As shown in 'man virsh' about schedinfo: Note: The cpu_shares parameter has a valid value range of 0-262144; Negative values are wrapped to positive, and larger values are capped at the maximum. Therefore, -1 is a useful shorthand for 262144. On the Linux kernel, the values 0 and 1 are automatically converted to a minimal value of 2. it works well with --live, but not with --config. Example: # virsh schedinfo rhel7-ful --set cpu_shares=0 --config Scheduler : posix cpu_shares : 0 vcpu_period: 0 vcpu_quota : 0 emulator_period: 0 emulator_quota : 0 cpu_shares is 0 rather than expected 2. What's worse, when we start it again, it is the default value of cpu_shares 1024. Because when we set the value of cpu_shares, when flag is --live, write the value into cgroup/cpu.shares. Then it will convert the value into the range of [2, 262144]. When flag is --config, we set the value into vmdef immidiately and 0 means no settting for cpu_shares. When we start vm again, libvirt use default value(1024) for it. commit bdffab0d5c52d31bd71422b0b69665efb6650953 Author: Ján Tomko jto...@redhat.com CommitDate: 2014-03-26 10:10:02 +0100 Treat zero cpu shares as a valid value changed this behavior. After this commit, if you set cpu_shares to 0 via schedinfo --config, the 0 will be written in the XML and used on domain startup. To use the default value, the shares element needs to be removed from the XML before restarting the domain, this cannot be currently done via 'virsh schedinfo'. Jan 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] qemu: Avoid leak in qemuDomainCheckRemoveOptionalDisk
On 05/15/2014 05:20 AM, Jiri Denemark wrote: Coverity complains about event being leaked in qemuDomainCheckRemoveOptionalDisk. The best fix for it is to avoid searching for the disk (using its srouce path even) we got by walking s/srouce/source/ through vm-def-disks array. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/qemu/qemu_domain.c | 25 ++--- 1 file changed, 10 insertions(+), 15 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing
On Tue, 6 May 2014 17:19:57 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote: On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote: On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote: On Wed, 30 Apr 2014 17:29:28 -0300 Eduardo Habkost ehabk...@redhat.com wrote: This series allows management code to use object-add on X86CPU subclasses, so it Is there any reason why device-add couldn't be used? It needs to work with -machine none, device_add requires a bus to exist, and there is no icc-bus on machine_none. The thing is that CPUID is a function of machine so using -machine none will provide only approximately accurate data. I'm not sure that retrieved possibly not accurate data are useful for libvirt. -cpu host doesn't depend on machine, and is the most important thing right now (because libvirt's checks for host QEMU/kernel/CPU capabilities is completely broken). true, but device-add/-cpu host could be used right now to get the same CPUID data wile using any machine type or default one without any of this patches. device_add can't be used with -machine none. Well, does caller need to use -machine in this case at all? Why not call QEMU with default machine type and get the same info using device_add ?? About machine-type data: currently libvirt has no concept of per-machine-type CPU model data, anyway. We (and libvirt) will need to address this eventually, but considering our track record, I bet the QEMU community will take a few years to finally provide that info to libvirt. I don't think QEMU is issue here, libvirt can use device-add to probe accurate CPUID on all CPU models on a given machine type now. libvirt should be fixed to care about machine type and use it to get correct CPUID data that QEMU provides. True, it should. But we still need a solution for the -cpu host problem. As you've said before '-cpu host' doesn't depend on machine type so libvirt could use device_add 'host-cpu' with default or any other PC machine type right now. In the meantime, we have a partial solution that fits the current libvirt data model and is better than the current state (where libvirt has to duplicate the CPU model data). Replacing one set of inaccurate CPUIDs with another is hardly solution. We could continue arguing about this, but let's ignore the issue about probing for CPU model information by now, and let's focus on host capability probing (-cpu host), then. How do you propose fixing that in a reasonable time (in QEMU 2.1) without requiring libvirt to re-run QEMU? Wouldn't -cpu host or alternative device_add command with default machine be sufficient? Maybe they will use this only for host, maybe they will use this for all the other CPU models, we are just providing the mechanism, it's their decision to use it or not. As I've said above libvirt could use device-add xxx-host or -cpu host to get it and second point I object to is providing yet another way to produce inaccurate CPUID info (libvirt has one already) and to do so hack [patches 1-3] CPU infrastructure to run out of context it's supposed to run in. While current API already allows to get correct CPUID data. IMHO, libvirt side should take advantage of information QEMU already provides. Current API requires re-running QEMU to query the information. This series allows it to be run with the -machine none QEMU instance that is already run by libvirt. The first thing I considered was making icc-bus user-creatable. Then I noticed it wouldn't work because object-add always add objects to /objects, not inside the qdev hierarchy (that's where device_add looks for the bus). So, allowing device_add could be possible, but would require changing more basic infrastructure: either allowing bus-less devices on device_add, or allowing device_add to add devices outside the qdev hierarchy, or allowing object-add to create objects outside /objects. Simply making CPU objects work with object-add was much simpler and less intrusive. And it had the interesting side-effect of _not_ doing things that are not required for CPU model probing (like creating an actual VCPU thread). can use it to probe for CPU model information without re-running QEMU. The main use case for this is to allow management code to create CPU objects and query the feature-words and filtered-features properties on the new objects, to find out which features each CPU
Re: [libvirt] [PATCH 1/3] virutil: Introduce a new macro named CLAMP.
On 05/15/2014 03:39 AM, Dongsheng Yang wrote:
This patch introduce a new macro to return a
value clamped to a given range.
[when sending a series, it's nice to include a cover letter with 'git
send-email --cover-letter to generate the 0/N message that all other
messages in the series reply to]
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/util/virutil.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 2bb74e2..e8536d8 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -37,6 +37,12 @@
# ifndef MAX
# define MAX(a, b) ((a) (b) ? (a) : (b))
# endif
+# ifndef CLAMP
+# define CLAMP(v, min, max) ({ \
This is gcc-specific. I'd rather avoid it, and stick to portable C99
code, if possible - which means doing this as an inline function rather
than a macro.
--
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] qemu: Avoid leak in qemuDomainCheckRemoveOptionalDisk
On Thu, May 15, 2014 at 01:20:39PM +0200, Jiri Denemark wrote:
Coverity complains about event being leaked in
qemuDomainCheckRemoveOptionalDisk. The best fix for it is to avoid
searching for the disk (using its srouce path even) we got by walking
s/srouce/source/ , but the whole sentence is hard to read, just The
best fix for it is to remove it by its index., and you can add ,
which is also safer if you want ;)
ACK with at lease the spelling fixed, it fixes an issue with multiple
disks using the same non-existing file path also.
Martin
through vm-def-disks array.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_domain.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7a0be12..78cfdc6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2176,11 +2176,11 @@ qemuDomainSetFakeReboot(virQEMUDriverPtr driver,
static int
qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- virDomainDiskDefPtr disk)
+ size_t diskIndex)
{
char uuid[VIR_UUID_STRING_BUFLEN];
virObjectEventPtr event = NULL;
-virDomainDiskDefPtr del_disk = NULL;
+virDomainDiskDefPtr disk = vm-def-disks[diskIndex];
const char *src = virDomainDiskGetSource(disk);
virUUIDFormat(vm-def-uuid, uuid);
@@ -2200,13 +2200,8 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr
driver,
event = virDomainEventDiskChangeNewFromObj(vm, src, NULL,
disk-info.alias,
VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START);
-
-if (!(del_disk = virDomainDiskRemoveByName(vm-def, src))) {
-virReportError(VIR_ERR_INVALID_ARG,
- _(no source device %s), src);
-return -1;
-}
-virDomainDiskDefFree(del_disk);
+virDomainDiskRemove(vm-def, diskIndex);
+virDomainDiskDefFree(disk);
}
if (event)
@@ -2218,11 +2213,11 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr
driver,
static int
qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- virDomainDiskDefPtr disk,
+ size_t diskIndex,
bool cold_boot)
{
char uuid[VIR_UUID_STRING_BUFLEN];
-int startupPolicy = disk-startupPolicy;
+int startupPolicy = vm-def-disks[diskIndex]-startupPolicy;
virUUIDFormat(vm-def-uuid, uuid);
@@ -2244,7 +2239,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
break;
}
-if (qemuDomainCheckRemoveOptionalDisk(driver, vm, disk) 0)
+if (qemuDomainCheckRemoveOptionalDisk(driver, vm, diskIndex) 0)
goto error;
return 0;
@@ -2282,11 +2277,11 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
{
int ret = -1;
size_t i;
-virDomainDiskDefPtr disk;
VIR_DEBUG(Checking for disk presence);
for (i = vm-def-ndisks; i 0; i--) {
-disk = vm-def-disks[i - 1];
+size_t idx = i - 1;
+virDomainDiskDefPtr disk = vm-def-disks[idx];
const char *path = virDomainDiskGetSource(disk);
virStorageFileFormat format = virDomainDiskGetFormat(disk);
virStorageType type = virStorageSourceGetActualType(disk-src);
@@ -2308,7 +2303,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
continue;
if (disk-startupPolicy
-qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
+qemuDomainCheckDiskStartupPolicy(driver, vm, idx,
cold_boot) = 0) {
virResetLastError();
continue;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu_driver: clamp value for setting sched cpu_shares with --config.
On 05/15/2014 03:39 AM, Dongsheng Yang wrote: As shown in 'man virsh' about schedinfo: Note: The cpu_shares parameter has a valid value range of 0-262144; This note documents historical kernel limits; if the kernel has changed, this may be out of date. Because when we set the value of cpu_shares, when flag is --live, write the value into cgroup/cpu.shares. Then it will convert the value into the range of [2, 262144]. When flag is --config, we set the value into vmdef immidiately and 0 means no settting for s/immidiately/immediately/ s/settting/setting/ cpu_shares. When we start vm again, libvirt use default value(1024) s/use/uses/ for it. This patch clamp the cpu_shares value when flag is --config, then s/clamp/clamps/ we will get then correct settting in output of virsh schedinfo s/settting/setting/ and value in cgroup after next booting of vm. +++ b/src/qemu/qemu_driver.c @@ -112,6 +112,8 @@ VIR_LOG_INIT(qemu.qemu_driver); #define QEMU_SCHED_MAX_PERIOD 100LL #define QEMU_SCHED_MIN_QUOTA 1000LL #define QEMU_SCHED_MAX_QUOTA 18446744073709551LL +#define QEMU_SCHED_MIN_SHARES 2LL +#define QEMU_SCHED_MAX_SHARES262144LL I'm a bit reluctant to add these numbers - if the kernel ever changes its range again (which HAS happened for some cgroup tunables), then we are needlessly preventing use of the newer range. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu_driver: clamp value for setting sched cpu_shares with --config.
On 05/15/2014 09:11 PM, Ján Tomko wrote: On 05/15/2014 11:39 AM, Dongsheng Yang wrote: As shown in 'man virsh' about schedinfo: Note: The cpu_shares parameter has a valid value range of 0-262144; Negative values are wrapped to positive, and larger values are capped at the maximum. Therefore, -1 is a useful shorthand for 262144. On the Linux kernel, the values 0 and 1 are automatically converted to a minimal value of 2. it works well with --live, but not with --config. Example: # virsh schedinfo rhel7-ful --set cpu_shares=0 --config Scheduler : posix cpu_shares : 0 vcpu_period: 0 vcpu_quota : 0 emulator_period: 0 emulator_quota : 0 cpu_shares is 0 rather than expected 2. What's worse, when we start it again, it is the default value of cpu_shares 1024. Because when we set the value of cpu_shares, when flag is --live, write the value into cgroup/cpu.shares. Then it will convert the value into the range of [2, 262144]. When flag is --config, we set the value into vmdef immidiately and 0 means no settting for cpu_shares. When we start vm again, libvirt use default value(1024) for it. commit bdffab0d5c52d31bd71422b0b69665efb6650953 Author: Ján Tomko jto...@redhat.com CommitDate: 2014-03-26 10:10:02 +0100 Treat zero cpu shares as a valid value changed this behavior. After this commit, if you set cpu_shares to 0 via schedinfo --config, the 0 will be written in the XML and used on domain startup. Indeed , it works. To use the default value, the shares element needs to be removed from the XML before restarting the domain, this cannot be currently done via 'virsh schedinfo'. And yes, my patch here is attempting to do the similar work in 'virsh schedinfo'. Also, it can make the other invalid input, such as -1 and 262144+1, get the expected output as manpage described. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] virutil: Introduce a new macro named CLAMP.
On 05/15/2014 09:14 PM, Eric Blake wrote:
On 05/15/2014 03:39 AM, Dongsheng Yang wrote:
This patch introduce a new macro to return a
value clamped to a given range.
[when sending a series, it's nice to include a cover letter with 'git
send-email --cover-letter to generate the 0/N message that all other
messages in the series reply to]
Okey, Thanx :)
Signed-off-by: Dongsheng Yang yangds.f...@cn.fujitsu.com
---
src/util/virutil.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 2bb74e2..e8536d8 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -37,6 +37,12 @@
# ifndef MAX
# define MAX(a, b) ((a) (b) ? (a) : (b))
# endif
+# ifndef CLAMP
+# define CLAMP(v, min, max) ({ \
This is gcc-specific. I'd rather avoid it, and stick to portable C99
code, if possible - which means doing this as an inline function rather
than a macro.
I prefer inline function too, but I found MAX and MIN are implemented
with macro, then
appended CLAMP to them.
Okey, I will use inline function in next version if this patch is
acceptable.
Thanx
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Avoid leak in qemuDomainCheckRemoveOptionalDisk
On 15.5.2014 13:20, Jiri Denemark wrote: Coverity complains about event being leaked in qemuDomainCheckRemoveOptionalDisk. The best fix for it is to avoid searching for the disk (using its srouce path even) we got by walking through vm-def-disks array. Signed-off-by: Jiri Denemark jdene...@redhat.com --- ACK with the spell fix :) Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC 1/5] cpu: Initialize cpu-stopped=true earlier
On Wed, 30 Apr 2014 17:29:29 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
In case something happens and prevents qemu_init_vcpu() from running
after cpu_exec_init() was already called, this will let the rest of the
VCPU handling code know that the CPU is not running yet.
perhaps default value should be set even earlier in
qom/cpu.c:cpu_common_initfn()
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
cpus.c | 1 -
exec.c | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/cpus.c b/cpus.c
index 7bbe153..69b0530 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1184,7 +1184,6 @@ void qemu_init_vcpu(CPUState *cpu)
{
cpu-nr_cores = smp_cores;
cpu-nr_threads = smp_threads;
-cpu-stopped = true;
if (kvm_enabled()) {
qemu_kvm_start_vcpu(cpu);
} else if (tcg_enabled()) {
diff --git a/exec.c b/exec.c
index 91513c6..e91decc 100644
--- a/exec.c
+++ b/exec.c
@@ -485,6 +485,7 @@ void cpu_exec_init(CPUArchState *env)
}
cpu-cpu_index = cpu_index;
cpu-numa_node = 0;
+cpu-stopped = true;
QTAILQ_INIT(cpu-breakpoints);
QTAILQ_INIT(cpu-watchpoints);
#ifndef CONFIG_USER_ONLY
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] virutil: Introduce a new macro named CLAMP.
On 05/15/2014 05:25 AM, Dongsheng Yang wrote:
+# define CLAMP(v, min, max) ({ \
This is gcc-specific. I'd rather avoid it, and stick to portable C99
code, if possible - which means doing this as an inline function rather
than a macro.
I prefer inline function too, but I found MAX and MIN are implemented
with macro, then
appended CLAMP to them.
Okey, I will use inline function in next version if this patch is
acceptable.
Martin's suggestion of using MIN(MAX()) is also C99 compliant, and
usable as a macro. For this particular code, a macro is preferable to
an inline function, because it is type-agnostic.
--
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] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing
On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber afaer...@suse.de wrote: Am 06.05.2014 22:19, schrieb Eduardo Habkost: On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote: On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote: On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote: On Wed, 30 Apr 2014 17:29:28 -0300 Eduardo Habkost ehabk...@redhat.com wrote: This series allows management code to use object-add on X86CPU subclasses, so it Is there any reason why device-add couldn't be used? It needs to work with -machine none, device_add requires a bus to exist, and there is no icc-bus on machine_none. The thing is that CPUID is a function of machine so using -machine none will provide only approximately accurate data. I'm not sure that retrieved possibly not accurate data are useful for libvirt. -cpu host doesn't depend on machine, and is the most important thing right now (because libvirt's checks for host QEMU/kernel/CPU capabilities is completely broken). true, but device-add/-cpu host could be used right now to get the same CPUID data wile using any machine type or default one without any of this patches. device_add can't be used with -machine none. I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1. PS: As side effect cpu/apic will disappear from info qtree HMP command output. I'm not aware of any real X86CPU dependency on ICCBus, so we might as well make -device place it on SysBus if no ICCBus is available... probably much more invasive and potentially dangerous though. Regards, Andreas -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu_driver: clamp value for setting sched cpu_shares with --config.
On 05/15/2014 05:18 AM, Dongsheng Yang wrote: Also, it can make the other invalid input, such as -1 and 262144+1, Auto-wrapping -1 to maximum makes sense. But making other out-of-bounds values, like 262144+1, be auto-clamped sounds risky, especially if the kernel ever changes clamping boundaries. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu_driver: clamp value for setting sched cpu_shares with --config.
On 05/15/2014 09:17 PM, Eric Blake wrote: On 05/15/2014 03:39 AM, Dongsheng Yang wrote: As shown in 'man virsh' about schedinfo: Note: The cpu_shares parameter has a valid value range of 0-262144; This note documents historical kernel limits; if the kernel has changed, this may be out of date. ... +++ b/src/qemu/qemu_driver.c @@ -112,6 +112,8 @@ VIR_LOG_INIT(qemu.qemu_driver); #define QEMU_SCHED_MAX_PERIOD 100LL #define QEMU_SCHED_MIN_QUOTA 1000LL #define QEMU_SCHED_MAX_QUOTA 18446744073709551LL +#define QEMU_SCHED_MIN_SHARES 2LL +#define QEMU_SCHED_MAX_SHARES262144LL I'm a bit reluctant to add these numbers - if the kernel ever changes its range again (which HAS happened for some cgroup tunables), then we are needlessly preventing use of the newer range. Yes, I hate these numbers too. But the range is defined in kernel/sched/sched.h #define MIN_SHARES (1UL 1) #define MAX_SHARES (1UL 18) and used in scheduler. shares = clamp(shares, scale_load(MIN_SHARES), scale_load(MAX_SHARES)); So we can not access it out from kernel. As I found there was some numbers for period and quota here, I added the shares number here. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing
On Thu, May 15, 2014 at 02:35:01PM +0200, Igor Mammedov wrote: On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber afaer...@suse.de wrote: Am 06.05.2014 22:19, schrieb Eduardo Habkost: On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote: On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote: On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote: On Wed, 30 Apr 2014 17:29:28 -0300 Eduardo Habkost ehabk...@redhat.com wrote: This series allows management code to use object-add on X86CPU subclasses, so it Is there any reason why device-add couldn't be used? It needs to work with -machine none, device_add requires a bus to exist, and there is no icc-bus on machine_none. The thing is that CPUID is a function of machine so using -machine none will provide only approximately accurate data. I'm not sure that retrieved possibly not accurate data are useful for libvirt. -cpu host doesn't depend on machine, and is the most important thing right now (because libvirt's checks for host QEMU/kernel/CPU capabilities is completely broken). true, but device-add/-cpu host could be used right now to get the same CPUID data wile using any machine type or default one without any of this patches. device_add can't be used with -machine none. I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1. Maybe there are no users of the current QOM path, but we do need a stable path to allow management to locate the CPU objects. Do we have one, already? -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing
Am 15.05.2014 15:07, schrieb Eduardo Habkost: On Thu, May 15, 2014 at 02:35:01PM +0200, Igor Mammedov wrote: On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber afaer...@suse.de wrote: Am 06.05.2014 22:19, schrieb Eduardo Habkost: On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote: On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote: On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote: On Wed, 30 Apr 2014 17:29:28 -0300 Eduardo Habkost ehabk...@redhat.com wrote: This series allows management code to use object-add on X86CPU subclasses, so it Is there any reason why device-add couldn't be used? It needs to work with -machine none, device_add requires a bus to exist, and there is no icc-bus on machine_none. The thing is that CPUID is a function of machine so using -machine none will provide only approximately accurate data. I'm not sure that retrieved possibly not accurate data are useful for libvirt. -cpu host doesn't depend on machine, and is the most important thing right now (because libvirt's checks for host QEMU/kernel/CPU capabilities is completely broken). true, but device-add/-cpu host could be used right now to get the same CPUID data wile using any machine type or default one without any of this patches. device_add can't be used with -machine none. I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1. Maybe there are no users of the current QOM path, but we do need a stable path to allow management to locate the CPU objects. Do we have one, already? No, we don't. That question is intertwined with topology modeling. :/ Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing
On Thu, May 15, 2014 at 02:14:18PM +0200, Igor Mammedov wrote: On Tue, 6 May 2014 17:19:57 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote: On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote: On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote: On Wed, 30 Apr 2014 17:29:28 -0300 Eduardo Habkost ehabk...@redhat.com wrote: This series allows management code to use object-add on X86CPU subclasses, so it Is there any reason why device-add couldn't be used? It needs to work with -machine none, device_add requires a bus to exist, and there is no icc-bus on machine_none. The thing is that CPUID is a function of machine so using -machine none will provide only approximately accurate data. I'm not sure that retrieved possibly not accurate data are useful for libvirt. -cpu host doesn't depend on machine, and is the most important thing right now (because libvirt's checks for host QEMU/kernel/CPU capabilities is completely broken). true, but device-add/-cpu host could be used right now to get the same CPUID data wile using any machine type or default one without any of this patches. device_add can't be used with -machine none. Well, does caller need to use -machine in this case at all? Why not call QEMU with default machine type and get the same info using device_add ?? I guess the explanation for -machine none is at: commit b4a738bf93c3137b92d532e59d60edccc4e1ea96 Author: Anthony Liguori aligu...@us.ibm.com Date: Wed Aug 22 15:22:05 2012 -0500 boards: add a 'none' machine type to all platforms This allows any QEMU binary to be executed with: $QEMU_BINARY -M none -qmp stdio Without errors from missing options that are required by various boards. This also provides a mode that we can use in the future to construct machines entirely through QMP commands. Cc: Daniel Berrange berra...@redhat.com Cc: Markus Armbruster arm...@redhat.com Signed-off-by: Anthony Liguori aligu...@us.ibm.com libvirt runs QEMU with -machine none before it knows anything about the QEMU binaries. There are many target architectures where the default machine-type won't work without extra options. 15 of the 26 qemu-system-* binaries on my Fedora 20 system failed to run as: $QEMU -nodefaults -no-user-config -nographic -monitor stdio (All of them ran happily when I added -M none.) About machine-type data: currently libvirt has no concept of per-machine-type CPU model data, anyway. We (and libvirt) will need to address this eventually, but considering our track record, I bet the QEMU community will take a few years to finally provide that info to libvirt. I don't think QEMU is issue here, libvirt can use device-add to probe accurate CPUID on all CPU models on a given machine type now. libvirt should be fixed to care about machine type and use it to get correct CPUID data that QEMU provides. True, it should. But we still need a solution for the -cpu host problem. As you've said before '-cpu host' doesn't depend on machine type so libvirt could use device_add 'host-cpu' with default or any other PC machine type right now. But not with -machine none. -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Avoid leak in qemuDomainCheckRemoveOptionalDisk
On Thu, May 15, 2014 at 14:16:51 +0200, Martin Kletzander wrote: On Thu, May 15, 2014 at 01:20:39PM +0200, Jiri Denemark wrote: Coverity complains about event being leaked in qemuDomainCheckRemoveOptionalDisk. The best fix for it is to avoid searching for the disk (using its srouce path even) we got by walking s/srouce/source/ , but the whole sentence is hard to read, just The best fix for it is to remove it by its index., and you can add , which is also safer if you want ;) ACK with at lease the spelling fixed, it fixes an issue with multiple disks using the same non-existing file path also. Thanks, I made the commit message better (hopefully) and pushed the patch. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing
On Thu, 15 May 2014 10:07:51 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Thu, May 15, 2014 at 02:35:01PM +0200, Igor Mammedov wrote: On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber afaer...@suse.de wrote: Am 06.05.2014 22:19, schrieb Eduardo Habkost: On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote: On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote: On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote: On Wed, 30 Apr 2014 17:29:28 -0300 Eduardo Habkost ehabk...@redhat.com wrote: This series allows management code to use object-add on X86CPU subclasses, so it Is there any reason why device-add couldn't be used? It needs to work with -machine none, device_add requires a bus to exist, and there is no icc-bus on machine_none. The thing is that CPUID is a function of machine so using -machine none will provide only approximately accurate data. I'm not sure that retrieved possibly not accurate data are useful for libvirt. -cpu host doesn't depend on machine, and is the most important thing right now (because libvirt's checks for host QEMU/kernel/CPU capabilities is completely broken). true, but device-add/-cpu host could be used right now to get the same CPUID data wile using any machine type or default one without any of this patches. device_add can't be used with -machine none. I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1. Maybe there are no users of the current QOM path, but we do need a stable path to allow management to locate the CPU objects. Do we have one, already? Can't we add query-cpus QMP command or something like this to hide path from user. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing
On Thu, May 15, 2014 at 03:48:16PM +0200, Igor Mammedov wrote: On Thu, 15 May 2014 10:07:51 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Thu, May 15, 2014 at 02:35:01PM +0200, Igor Mammedov wrote: On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber afaer...@suse.de wrote: Am 06.05.2014 22:19, schrieb Eduardo Habkost: On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote: On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote: On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote: On Wed, 30 Apr 2014 17:29:28 -0300 Eduardo Habkost ehabk...@redhat.com wrote: This series allows management code to use object-add on X86CPU subclasses, so it Is there any reason why device-add couldn't be used? It needs to work with -machine none, device_add requires a bus to exist, and there is no icc-bus on machine_none. The thing is that CPUID is a function of machine so using -machine none will provide only approximately accurate data. I'm not sure that retrieved possibly not accurate data are useful for libvirt. -cpu host doesn't depend on machine, and is the most important thing right now (because libvirt's checks for host QEMU/kernel/CPU capabilities is completely broken). true, but device-add/-cpu host could be used right now to get the same CPUID data wile using any machine type or default one without any of this patches. device_add can't be used with -machine none. I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1. Maybe there are no users of the current QOM path, but we do need a stable path to allow management to locate the CPU objects. Do we have one, already? Can't we add query-cpus QMP command or something like this to hide path from user. That would work, too. But why is a dedicated query-cpus command better than something like qom-list path=/machine/cpus (that could simply return a list of links to the actual CPU objects)? -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv5 2/3] virsh: Expose virDomain{Get,Set}Time
These APIs are exposed under new virsh command 'domtime' which both gets
and sets (not at the same time of course :)).
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
diff to v4:
- changed wording in some help messages
- use VSH_EXCLUSIVE_OPTIONS instead explicit separate code
tools/virsh-domain-monitor.c | 116 +++
tools/virsh.pod | 18 +++
2 files changed, 134 insertions(+)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 18d551a..a3b66ed 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1356,6 +1356,116 @@ cmdDomstate(vshControl *ctl, const vshCmd *cmd)
}
/*
+ * domtime command
+ */
+static const vshCmdInfo info_domtime[] = {
+{.name = help,
+ .data = N_(domain time)
+},
+{.name = desc,
+ .data = N_(Gets or sets the domain's system time)
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_domtime[] = {
+{.name = domain,
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_(domain name, id or uuid)
+},
+{.name = now,
+ .type = VSH_OT_BOOL,
+ .help = N_(set to the time of the host running virsh)
+},
+{.name = pretty,
+ .type = VSH_OT_BOOL,
+ .help = N_(print domain's time in human readable form)
+},
+{.name = sync,
+ .type = VSH_OT_BOOL,
+ .help = N_(instead of setting given time, synchronize from domain's
RTC),
+},
+{.name = time,
+ .type = VSH_OT_INT,
+ .help = N_(time to set)
+},
+{.name = NULL}
+};
+
+static bool
+cmdDomTime(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+bool ret = false;
+bool now = vshCommandOptBool(cmd, now);
+bool pretty = vshCommandOptBool(cmd, pretty);
+bool sync = vshCommandOptBool(cmd, sync);
+long long seconds = 0;
+unsigned int nseconds = 0;
+unsigned int flags = 0;
+bool doSet = false;
+int rv;
+
+VSH_EXCLUSIVE_OPTIONS(time, now);
+VSH_EXCLUSIVE_OPTIONS(time, sync);
+VSH_EXCLUSIVE_OPTIONS(now, sync);
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+rv = vshCommandOptLongLong(cmd, time, seconds);
+
+if (rv 0) {
+/* invalid integer format */
+vshError(ctl, %s,
+ _(Unable to parse integer parameter to --time.));
+goto cleanup;
+} else if (rv 0) {
+/* valid integer to set */
+doSet = true;
+}
+
+if (doSet || now || sync) {
+if (now ((seconds = time(NULL)) == (time_t) -1)) {
+vshError(ctl, _(Unable to get current time));
+goto cleanup;
+}
+
+if (sync)
+flags |= VIR_DOMAIN_TIME_SYNC;
+
+if (virDomainSetTime(dom, seconds, nseconds, flags) 0)
+goto cleanup;
+
+} else {
+if (virDomainGetTime(dom, seconds, nseconds, flags) 0)
+goto cleanup;
+
+if (pretty) {
+char timestr[100];
+time_t cur_time = seconds;
+struct tm time_info;
+
+if (!gmtime_r(cur_time, time_info)) {
+vshError(ctl, _(Unable to format time));
+goto cleanup;
+}
+strftime(timestr, sizeof(timestr), %Y-%m-%d %H:%M:%S,
time_info);
+
+vshPrint(ctl, _(Time: %s), timestr);
+} else {
+vshPrint(ctl, _(Time: %lld), seconds);
+}
+}
+
+ret = true;
+ cleanup:
+virDomainFree(dom);
+return ret;
+}
+
+/*
* list command
*/
static const vshCmdInfo info_list[] = {
@@ -1911,6 +2021,12 @@ const vshCmdDef domMonitoringCmds[] = {
.info = info_domstate,
.flags = 0
},
+{.name = domtime,
+ .handler = cmdDomTime,
+ .opts = opts_domtime,
+ .info = info_domtime,
+ .flags = 0
+},
{.name = list,
.handler = cmdList,
.opts = opts_list,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 22ca196..120715a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1017,6 +1017,24 @@ Returns state of an interface to VMM used to control a
domain. For
states other than ok or error the command also prints number of
seconds elapsed since the control interface entered its current state.
+=item Bdomtime Idomain { [I--now] [I--pretty] [I--sync]
+[I--time Btime] }
+
+Gets or sets the domain's system time. When run without any arguments
+(but Idomain), the current domain's system time is printed out. The
+I--pretty modifier can be used to print the time in more human
+readable form.
+
+When I--time Btime is specified, the domain's time is
+not get but set instead. The I--now modifier acts like if it was an
+alias for I--time B$now, which means it sets the time that is
+currently on the host virsh is running at. In both cases (setting and
+getting), time is in seconds relative to Epoch of 1970-01-01 in UTC.
+The I--sync modifies the set behavior a bit: The time passed is
+ignored, but the time
[libvirt] [PATCHv5 3/3] qemu: Implement virDomain{Get,Set}Time
One caveat though, qemu-ga is expecting time and returning time
in nanoseconds. With all the buffering and propagation delay, the
time is already wrong once it gets to the qemu-ga, but there's
nothing we can do about it.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
diff to v4:
- s/1.2.4/1.2.5/
- drop explicit agent error check since it's done in qemuAgentCommand now
src/qemu/qemu_agent.c | 93 +
src/qemu/qemu_agent.h | 8
src/qemu/qemu_driver.c | 109 +
3 files changed, 210 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 57c7cc5..10e2b8d 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1685,3 +1685,96 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus,
return 0;
}
+
+
+int
+qemuAgentGetTime(qemuAgentPtr mon,
+ long long *seconds,
+ unsigned int *nseconds)
+{
+int ret = -1;
+unsigned long long json_time;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+cmd = qemuAgentMakeCommand(guest-get-time,
+ NULL);
+if (!cmd)
+return ret;
+
+if (qemuAgentCommand(mon, cmd, reply, true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) 0)
+goto cleanup;
+
+if (virJSONValueObjectGetNumberUlong(reply, return, json_time) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(malformed return value));
+goto cleanup;
+}
+
+/* guest agent returns time in nanoseconds,
+ * we need it in seconds here */
+*seconds = json_time / 10LL;
+*nseconds = json_time % 10LL;
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
+
+
+/**
+ * qemuAgentSetTime:
+ * @setTime: time to set
+ * @sync: let guest agent to read domain's RTC (@setTime is ignored)
+ */
+int
+qemuAgentSetTime(qemuAgentPtr mon,
+long long seconds,
+unsigned int nseconds,
+bool sync)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+if (sync) {
+cmd = qemuAgentMakeCommand(guest-set-time, NULL);
+} else {
+/* guest agent expect time with nanosecond granularity.
+ * Impressing. */
+long long json_time;
+
+/* Check if we overflow. For some reason qemu doesn't handle unsigned
+ * long long on the monitor well as it silently truncates numbers to
+ * signed long long. Therefore we must check overflow against LLONG_MAX
+ * not ULLONG_MAX. */
+if (seconds LLONG_MAX / 10LL) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(Time '%lld' is too big for guest agent),
+ seconds);
+return ret;
+}
+
+json_time = seconds * 10LL;
+json_time += nseconds;
+cmd = qemuAgentMakeCommand(guest-set-time,
+ I:time, json_time,
+ NULL);
+}
+
+if (!cmd)
+return ret;
+
+if (qemuAgentCommand(mon, cmd, reply, true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 58531d5..6cd6b49 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -98,4 +98,12 @@ int qemuAgentSetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr
cpus, size_t ncpus);
int qemuAgentUpdateCPUInfo(unsigned int nvcpus,
qemuAgentCPUInfoPtr cpuinfo,
int ncpuinfo);
+
+int qemuAgentGetTime(qemuAgentPtr mon,
+ long long *seconds,
+ unsigned int *nseconds);
+int qemuAgentSetTime(qemuAgentPtr mon,
+ long long seconds,
+ unsigned int nseconds,
+ bool sync);
#endif /* __QEMU_AGENT_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 52ca47c..cab653b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16536,6 +16536,113 @@ qemuConnectGetCPUModelNames(virConnectPtr conn,
return cpuGetModels(arch, models);
}
+static int
+qemuDomainGetTime(virDomainPtr dom,
+ long long *seconds,
+ unsigned int *nseconds,
+ unsigned int flags)
+{
+virQEMUDriverPtr driver = dom-conn-privateData;
+virDomainObjPtr vm = NULL;
+qemuDomainObjPrivatePtr priv;
+int ret = -1;
+int rv;
+
+virCheckFlags(0, ret);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return ret;
+
+if (virDomainGetTimeEnsureACL(dom-conn, vm-def) 0)
+goto cleanup;
+
+priv = vm-privateData;
+
+if
Re: [libvirt] [PATCHv5 3/3] qemu: Implement virDomain{Get,Set}Time
On 05/15/2014 04:13 PM, Michal Privoznik wrote: One caveat though, qemu-ga is expecting time and returning time in nanoseconds. With all the buffering and propagation delay, the time is already wrong once it gets to the qemu-ga, but there's nothing we can do about it. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- diff to v4: - s/1.2.4/1.2.5/ - drop explicit agent error check since it's done in qemuAgentCommand now src/qemu/qemu_agent.c | 93 + src/qemu/qemu_agent.h | 8 src/qemu/qemu_driver.c | 109 + 3 files changed, 210 insertions(+) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 2/3] virsh: Expose virDomain{Get,Set}Time
On 05/15/2014 04:13 PM, Michal Privoznik wrote: These APIs are exposed under new virsh command 'domtime' which both gets and sets (not at the same time of course :)). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- diff to v4: - changed wording in some help messages - use VSH_EXCLUSIVE_OPTIONS instead explicit separate code tools/virsh-domain-monitor.c | 116 +++ tools/virsh.pod | 18 +++ 2 files changed, 134 insertions(+) + +Gets or sets the domain's system time. When run without any arguments +(but Idomain), the current domain's system time is printed out. The +I--pretty modifier can be used to print the time in more human +readable form. + +When I--time Btime is specified, the domain's time is +not get but set instead. The I--now modifier acts like if it was an s/get/gotten/ I think +alias for I--time B$now, which means it sets the time that is +currently on the host virsh is running at. In both cases (setting and +getting), time is in seconds relative to Epoch of 1970-01-01 in UTC. +The I--sync modifies the set behavior a bit: The time passed is +ignored, but the time to set is read from domain's RTC instead. Please +note, that some hypervisors may require a guest agent to be configured +in order to get or set the guest time. + =item Bdomxml-from-native Iformat Iconfig Convert the file Iconfig in the native guest configuration format ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Minor chardev fixes
Ján Tomko (3): Remove chardev port calculation from DomainDefParse Move console target port setting to DeviceDefPostParse Create a console stub for the first serial device src/conf/domain_conf.c | 77 -- 1 file changed, 30 insertions(+), 47 deletions(-) -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Remove chardev port calculation from DomainDefParse
It's already in DeviceDefPostParse, the only difference is
that the removed code only looked at ports of devices
before the current one, the new code looks at all of them.
---
src/conf/domain_conf.c | 33 -
1 file changed, 33 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c3bdad..46294fa 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12355,15 +12355,6 @@ virDomainDefParseXML(xmlDocPtr xml,
if (!chr)
goto error;
-if (chr-target.port == -1) {
-int maxport = -1;
-size_t j;
-for (j = 0; j i; j++) {
-if (def-parallels[j]-target.port maxport)
-maxport = def-parallels[j]-target.port;
-}
-chr-target.port = maxport + 1;
-}
def-parallels[def-nparallels++] = chr;
}
VIR_FREE(nodes);
@@ -12383,15 +12374,6 @@ virDomainDefParseXML(xmlDocPtr xml,
if (!chr)
goto error;
-if (chr-target.port == -1) {
-int maxport = -1;
-size_t j;
-for (j = 0; j i; j++) {
-if (def-serials[j]-target.port maxport)
-maxport = def-serials[j]-target.port;
-}
-chr-target.port = maxport + 1;
-}
def-serials[def-nserials++] = chr;
}
VIR_FREE(nodes);
@@ -12439,21 +12421,6 @@ virDomainDefParseXML(xmlDocPtr xml,
chr-targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO
chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
chr-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL;
-
-if (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-chr-info.addr.vioserial.port == 0) {
-int maxport = 0;
-size_t j;
-for (j = 0; j i; j++) {
-virDomainChrDefPtr thischr = def-channels[j];
-if (thischr-info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
-thischr-info.addr.vioserial.controller ==
chr-info.addr.vioserial.controller
-thischr-info.addr.vioserial.bus ==
chr-info.addr.vioserial.bus
-(int)thischr-info.addr.vioserial.port maxport)
-maxport = thischr-info.addr.vioserial.port;
-}
-chr-info.addr.vioserial.port = maxport + 1;
-}
}
VIR_FREE(nodes);
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] Create a console stub for the first serial device
The console alias for the first serial device was only
formatted when there were no consoles.
After removing this alias manually, a round-trip via XML
would add it again. However this was not the case when
it was removed and a virtio console was hotplugged.
https://bugzilla.redhat.com/show_bug.cgi?id=1089914
---
src/conf/domain_conf.c | 30 --
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3c0d2ff..69106bd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2923,6 +2923,26 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
}
}
+/* Create a stub for the first serial device in consoles if there are none
*/
+if (STREQ(def-os.type, hvm)
+def-nconsoles == 0
+def-nserials 0) {
+
+virDomainChrDefPtr chr;
+
+if (VIR_ALLOC(chr) 0)
+return -1;
+
+if (VIR_APPEND_ELEMENT(def-consoles,
+ def-nconsoles,
+ chr) 0) {
+VIR_FREE(chr);
+return -1;
+}
+def-consoles[0]-deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
+def-consoles[0]-targetType =
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
+}
+
if (virDomainDefRejectDuplicateControllers(def) 0)
return -1;
@@ -17739,16 +17759,6 @@ virDomainDefFormatInternal(virDomainDefPtr def,
if (virDomainChrDefFormat(buf, console, flags) 0)
goto error;
}
-if (STREQ(def-os.type, hvm)
-def-nconsoles == 0
-def-nserials 0) {
-virDomainChrDef console;
-memcpy(console, def-serials[n], sizeof(console));
-console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
-console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
-if (virDomainChrDefFormat(buf, console, flags) 0)
-goto error;
-}
for (n = 0; n def-nchannels; n++)
if (virDomainChrDefFormat(buf, def-channels[n], flags) 0)
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Move console target port setting to DeviceDefPostParse
This overrides the port number on hotplug, not just
when parsing the domain XML.
https://bugzilla.redhat.com/show_bug.cgi?id=1089991
https://bugzilla.redhat.com/show_bug.cgi?id=1089997
---
src/conf/domain_conf.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 46294fa..3c0d2ff 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2993,18 +2993,25 @@
virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
if (dev-type == VIR_DOMAIN_DEVICE_CHR) {
virDomainChrDefPtr chr = dev-data.chr;
const virDomainChrDef **arrPtr;
-size_t i, cnt;
+size_t i, cnt, idx;
virDomainChrGetDomainPtrs(def, chr-deviceType, arrPtr, cnt);
+for (idx = 0; idx cnt; idx++) {
+if (arrPtr[idx] == chr)
+break;
+}
+
if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE
chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)
chr-targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
+if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE)
+chr-target.port = idx;
+
if (chr-target.port == -1
(chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL ||
- chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL ||
- chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE)) {
+ chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL)) {
int maxport = -1;
for (i = 0; i cnt; i++) {
@@ -12395,7 +12402,6 @@ virDomainDefParseXML(xmlDocPtr xml,
if (!chr)
goto error;
-chr-target.port = i;
def-consoles[def-nconsoles++] = chr;
}
VIR_FREE(nodes);
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5] Add helper program to create custom leases
On Sat, Apr 26, 2014 at 5:29 AM, Nehal J Wani nehaljw.k...@gmail.com wrote:
Introduce helper program to catch events from dnsmasq and maintain a custom
lease file per network. It supports dhcpv4 and dhcpv6. The file is saved as
interface-name.status.
Each lease contains the following info:
expiry-time (epoch time) mac iaid ip-address hostname clientid
Example of custom leases file content:
[
{
iaid: 1221229,
ip-address: 2001:db8:ca2:2:1::95,
mac-address: 52:54:00:12:a2:6d,
hostname: Fedora20,
client-id: 00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55,
expiry-time: 1393244216
},
{
ip-address: 192.168.150.208,
mac-address: 52:54:00:11:56:b3,
hostname: Wani-PC,
client-id: 01:52:54:00:11:56:b3,
expiry-time: 1393244248
}
]
src/Makefile.am:
* Add options to compile the helper program
src/network/bridge_driver.c:
* Introduce networkDnsmasqLeaseFileNameCustom()
* Invoke helper program along with dnsmasq
* Delete the .status file when corresponding n/w is destroyed.
src/network/leaseshelper.c
* Helper program to create the custom lease file
---
v5:
* More comments added, for better explanation
* Use of virFileFindResource() to identify proper path to helper binary
* Use of VIR_ENUM_IMPL for handling action events added
v4:
* Addition of pidfile and a corresponding lock for it
* Make correction for dnsmasq 2.52 (Only IPv4)
* Move helper file from src/util to src/network
* Increase limit on max size of leases file
* Refer: https://www.redhat.com/archives/libvir-list/2014-March/msg01038.html
v3:
* Improved file handling, removed redundant copying, introduced --help and
--version
* Refer:
https://www.redhat.com/archives/libvir-list/2014-February/msg01431.html
v2:
* Changed format to JSON
* Refer:
https://www.redhat.com/archives/libvir-list/2014-January/msg01234.html
v1:
* Refer:
https://www.redhat.com/archives/libvir-list/2014-January/msg00626.html
src/Makefile.am | 22 +++
src/network/bridge_driver.c | 27
src/network/leaseshelper.c | 360
+++
3 files changed, 409 insertions(+), 0 deletions(-)
create mode 100644 src/network/leaseshelper.c
Ping!
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] require for suggestions on support for ivshmem device
Thank you for bringing this up. I'm not experienced with the inner workings of libvirt, but I'm happy to help in anyway I can in terms of clarifying ivshmem's behaviour. Cheers, Cam On Wed, May 14, 2014 at 2:23 AM, Wangrui (K) moon.wang...@huawei.comwrote: Hi, Libvirt does not support ivshmem(Inter-VM Shared Memory) device recently, thus, I would like to know if there's any plan to support it in the future? If not, I would like to contribute a serial of patches to do so. On Jan 28, Wangyufei (James) asked about this question, and Daniel replied with 2 suggestions: 1. Libvirt should be capable of configuring the guest's xml on ivshmem. 2.An ivshmem daemon is needed to run on the host to support it, libvirt is recommended to provide such a daemon. Please refer to https://www.redhat.com/archives/libvir-list/2014-January/msg01335.htmlfor details. What I'll do later is the 1st suggestion, the 2nd one is left to be accomplished by someone else. Here is the detailed work I'll do to support configuration of the guest in libvirt: virDomainDefParseXML: parse ivshmem device xml when defining dom.xml virDomainDeviceInfoIterateInternal: iterate ivshmem devices qemuAssignDevicePCISlots: assign ivshmem device pci slots virDomainDefFormatInternal: format ivshmem device xml (Eg. virsh edit dom) virDomainDefFree: free ivshmem device def qemuBuildCommandLine: build ivshmem device command line when vm starts qemuAssignDeviceAliases: assign ivshmem device aliases when vm starts virDomainDeviceDefParse: attach and parse ivshmem device xml qemuDomainAttachDeviceConfig: attach ivshmem device xml persistently qemuDomainAttachDeviceLive: attach ivshmem device online qemuDomainDetachDeviceConfig: detach ivshmem device xml persistently qemuDomainDetachDeviceLive: detach ivshmem device online There are two ways to use ivshmem with qemu (please refer to http://qemu.weilnetz.de/qemu-doc.html#pcsys_005fother_005fdevs ): 1.Guests map a POSIX shared memory region into the guest as a PCI device that enables zero-copy communication to the application level of the guests, The basic syntax is: qemu-system-i386-device ivshmem, size = size in format accepted by -m [, shm = shm name] 2.If desired, interrupts can be sent between guest VMs accessing the same shared memory region. Interrupt support requires using a shared memory server and using a chardev socket to connect to it. An example syntax when using the shared memory server is: qemu-system-i386-device ivshmem, size = size in format accepted by -m [, chardev = id] [, msi = on] [, ioeventfd = on] [, vectors = n] [, role = peer | master] qemu-system-i386-chardev socket, path = path, id = id The respective xml configuration for the above 2 qemu command lines are shown as below: Example1: automatically attach device with KVM devices ivshmem role='master' memory name='dom-ivshmem' size='2'/ /ivshmem /devices NOTE: size means ivshmem size in unit MB, name means shm name role is optional, it may be set to master or peer, the default one is master Example2: manually attach device with static PCI slot 4 requested devices ivshmem role='master' memory name='dom-ivshmem' size='2'/ address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0'/ /ivshmem /devices Example3: automatically attach device with KVM devices ivshmem role='master' type='unix' source mode='connect' path='/tmp/ivshmem'/ memory name='dom-ivshmem' size='2'/ /ivshmem /devices NOTE: path means shared memory socket path which is set by the daemon. source mode and type is similar with vmchannel. I'm looking forward to your suggestions, thank you very much. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] Don't log an internal error when the guest hasn't updated balloon stats
On 05/15/2014 01:22 AM, Ján Tomko wrote:
If virDomainMemoryStats is called too soon after domain startup,
QEMU returns:
error:{class:GenericError,desc:guest hasn't updated any stats yet}
when we try to query balloon stats.
Check for this reply and log it as OPERATION_INVALID instead of
INTERNAL_ERROR. This means the daemon only logs it at the debug level,
without polluting system logs.
Reported by Laszlo Pal:
https://www.redhat.com/archives/libvirt-users/2014-May/msg00023.html
---
v1: https://www.redhat.com/archives/libvir-list/2014-May/msg00420.html
v2:
return 0 in this case - even though balloon stats are not yet available,
we can still return 'rss' in qemuDomainMemoryStats
jump to cleanup if CheckError returns 0
src/qemu/qemu_monitor_json.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
+if ((data = virJSONValueObjectGet(reply, error))) {
+const char *klass = virJSONValueObjectGetString(data, class);
+const char *desc = virJSONValueObjectGetString(data, desc);
-if (ret 0)
+if (STREQ_NULLABLE(klass, GenericError)
+STREQ_NULLABLE(desc, guest hasn't updated any stats yet)) {
Adding qemu. Uggh - the qemu documentation of QMP states:
- The desc member is a human-readable error message. Clients should
not attempt to parse this message.
because the contents of that field are NOT guaranteed to be stable.
We're stuck parsing that field for old versions of qemu, but this is one
case where upstream qemu (for future versions) should change the class
member of that particular error case to a distinct value other than
GenericError so that it is trivially obvious when this particular
condition has occurred, since it is a case where libvirt wants to treat
it as a non-error.
reluctant ACK, while hoping that we can do something more reliable in
the future.
--
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] Hyper-V 2012 R2 support
Hi,
This patch adds support to libvirt to work with Hyper-V 2012 R2. This
will however mean
that Hyper-V 2008 will no longer be supported. Microsoft supports
hyperv 2008 management
via windows 7 and hyperv 2012 via windows 8.1 (note that the reverse
is not true, that is,
win 7 cannot manage hyperv 2012 and win 8.1 cannot manage hyperv
2008). I think they wanted
to keep the application code separate, in the sense that one manages
the old namespace and
the new one manages the new v2 namespace. The difference between the
two namespaces is not great
either, sadly, both of them have same class names but with different
field types (int, string etc.)
which have been separated by the namespace root/virtualization vs
root/virtualization/v2
For users who wish to use libvirt to manage Hyper-V 2008, using an
older version of libvirt = 1.4.2 should work.
Since new Hyper-V 2008 drivers wont be contributed to libvirt anyway,
so it is not like the users will be missing on features.
And it is more likely that all future contributions to libvirt will be
on Hyper-V 2012 R2 and beyond.
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index aed9307..0253496 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -893,8 +893,7 @@ hypervDomainGetXMLDesc(virDomainPtr domain,
unsigned int flags)
if (VIR_STRDUP(def-name, computerSystem-data-ElementName) 0)
goto cleanup;
-if (VIR_STRDUP(def-description,
virtualSystemSettingData-data-Notes) 0)
-goto cleanup;
+// No need to check length of Notes, it is now a dynamic array
def-mem.max_balloon = memorySettingData-data-Limit * 1024; /*
megabyte to kilobyte */
def-mem.cur_balloon = memorySettingData-data-VirtualQuantity *
1024; /* megabyte to kilobyte */
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index 6e6f629..4c9ad34 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -39,7 +39,7 @@
http://schemas.microsoft.com/wbem/wsman/1/wmi/root/cimv2/*;
#define ROOT_VIRTUALIZATION \
-http://schemas.microsoft.com/wbem/wsman/1/wmi/root/virtualization/*;
+http://schemas.microsoft.com/wbem/wsman/1/wmi/root/virtualization/v2/*;
#define VIR_FROM_THIS VIR_FROM_HYPERV
diff --git a/src/hyperv/hyperv_wmi_generator.input
b/src/hyperv/hyperv_wmi_generator.input
index 97f9dff..bb0e8c1 100644
--- a/src/hyperv/hyperv_wmi_generator.input
+++ b/src/hyperv/hyperv_wmi_generator.input
@@ -20,6 +20,9 @@
#
# Based on MSDN Hyper-V WMI Classes:
# http://msdn.microsoft.com/en-us/library/cc136986%28v=vs.85%29.aspx
+# Hyper-V 2012 version (root/virtualization/v2 namespace):
http://msdn.microsoft.com/en-us/library/hh850257(v=vs.85).aspx
+# + Currently, Classes {Msvm_MemorySettingData,
Msvm_ProcessorSettingData, Msvm_VirtualSystemSettingData} are in v2
namespace
+# + Will eventually need to upgrade the remainder of the
classes as well.
#
@@ -97,80 +100,90 @@ end
class Msvm_MemorySettingData
-string Caption
-string Description
-string InstanceID
-string ElementName
-uint16 ResourceType
-string OtherResourceType
-string ResourceSubType
-string PoolID
-uint16 ConsumerVisibility
-string HostResource[]
-string AllocationUnits
-uint64 VirtualQuantity
-uint64 Reservation
-uint64 Limit
-uint32 Weight
-boolean AutomaticAllocation
-boolean AutomaticDeallocation
-string Parent
-string Connection[]
-string Address
-uint16 MappingBehavior
-boolean IsVirtualized
-string DeviceID
-string DeviceIDFormat
-boolean DynamicMemoryEnabled
-#uint32 TargetMemoryBuffer # Available only on Windows Server 2008 R2 SP1
+string InstanceID
+string Caption
+string Description
+string ElementName
+uint16 ResourceType
+string OtherResourceType
+string ResourceSubType
+string PoolID
+uint16 ConsumerVisibility
+string HostResource[]
+string AllocationUnits
+uint64 VirtualQuantity
+uint64 Reservation
+uint64 Limit
+uint32 Weight
+boolean AutomaticAllocation
+boolean AutomaticDeallocation
+string Parent
+string Connection[]
+string Address
+uint16 MappingBehavior
+string AddressOnParent
+string VirtualQuantityUnits
+boolean DynamicMemoryEnabled
+uint32 TargetMemoryBuffer
+boolean IsVirtualized
+boolean SwapFilesInUse
+uint64 MaxMemoryBlocksPerNumaNode
end
class Msvm_ProcessorSettingData
-string Caption
-string Description
-string InstanceID
-string ElementName
-uint16 ResourceType
-string OtherResourceType
-string ResourceSubType
-string PoolID
-uint16 ConsumerVisibility
-string HostResource[]
-string AllocationUnits
-uint64 VirtualQuantity
-uint64 Reservation
-uint64 Limit
-uint32 Weight
-boolean
Re: [libvirt] [PATCH] qemu: Fix specifying char devs for PPC
Ping. This is a patch similar with ARM platforms. http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=3a2beaee1d50dc968171c584ec2edcfdcb8fadde Right now on ppce500, chardev is not supported for the serial console. So it uses the the legacy -serial option. Best Regards, Olivia -Original Message- From: Olivia Yin [mailto:hong-hua@freescale.com] Sent: Wednesday, May 14, 2014 6:47 PM To: libvir-list@redhat.com Cc: Yin Olivia-R63875 Subject: [PATCH] qemu: Fix specifying char devs for PPC QEMU ppce500 board uses the old style -serial options. Other PPC boards don't give any way to explicitly wire in a -chardev except pseries which uses -device spapr-vty with -chardev. --- src/qemu/qemu_capabilities.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b491f58..fe5dd19 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3460,13 +3460,17 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; -if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64)) +if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64) + (def-os.arch != VIR_ARCH_PPC) (def-os.arch != +VIR_ARCH_PPC64)) return true; /* This may not be true for all ARM machine types, but at least * the only supported non-virtio serial devices of vexpress and versatile - * don't have the -chardev property wired up. */ + * don't have the -chardev property wired up. + * For PPC machines, only pserial need -device spapr-vty with + -chardev */ return (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO || (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE - chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); + chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) || +(chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL + chr-info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)); } -- 1.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu_driver: clamp value for setting sched cpu_shares with --config.
On 05/15/2014 09:20 PM, Eric Blake wrote: On 05/15/2014 05:18 AM, Dongsheng Yang wrote: Also, it can make the other invalid input, such as -1 and 262144+1, Auto-wrapping -1 to maximum makes sense. Actually, -1 is not the minmum-1, because the range is [2, 262144]. The reason -1 is converted to maxmum is that -1 is treated as unsigned long of 2^64-1. Then clamp it to range is 262144. But making other out-of-bounds values, like 262144+1, be auto-clamped sounds risky, especially if the kernel ever changes clamping boundaries. There are two approaches for this issue I think. (1), use SCHED_RANGE_CHECK() for cpu_shares, same with period and quota, then if the value is our of the range, raise an error. (2), keep the behavior for out-of-bounds values in --config as what it is in --live. --live is depending on the conversion of cgroup, then we should follow the style of cgroup in --config too, right? It means 262144+1 should clamped to maxmum. About the concern you mentioned that boundaries may be changed in future, as I explained in another mail in this thread, it is defined in private zone of scheduler, I can not catch up with a good idea to solve it. :( -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [Question] Collect vNode pinning to pNode run-time information
Hi all, I have a question about NUMA. User configured vNode(guest virtual numa node), but he didn't configure cputune and numatune. Now we want to get the information that each vNode run in which pNode(host numa node). It's run-time information that may be modified with high frequency. In current Libvirt's API, we can get the information that each vCpu running on which pCpu through virsh vcpuinfo(there should be a corresponding Libvirt API function). But we didn't find any APIs to get the information that each vNode uses which pNode's memory, or just each vCpu consumes which pNode's memory. We find a command numastat -mcn -p qemu that can get the memory consume data of each VM, but it still loses the information that we want(vNode memory consume data), as following: # numastat -mcn -p qemu Per-node process memory usage (in MBs) PID Node 0 Node 1 Total --- -- -- - 8900 (qemu-kvm)2032 50 2083 17716 (qemu-kvm) 1546663 2209 22484 (qemu-kvm)621 1524 2146 29694 (qemu-kvm)892 1350 2242 --- -- -- - Total 5092 3588 8680 . My question is: 1. In Libvirt, are there any ways that we can get our needed data? 2. If no ways in Libvirt, do you have any other suggestions to collect the information? Any feedbacks from you are appreciated. Thanks Best Regards Shi, Xiao-Lei (Bruce) Hewlett-Packard Co., Ltd. HP Servers Core Platform Software China Telephone +86 23 65683093 Mobile +86 18696583447 Email xiao-lei@hp.commailto:shiguo...@hp.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 6/7] security_dac: avoid relabeling hostdevs when relabel='no'
When relabel='no' at the domain level, there is no need to call
the hostdev relabeling functions.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/security/security_dac.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index d6ca303..4434cd0 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -485,6 +485,9 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr
mgr,
cbdata.manager = mgr;
cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+if (cbdata.secdef cbdata.secdef-norelabel)
+return 0;
+
switch ((enum virDomainHostdevSubsysType) dev-source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
virUSBDevicePtr usb;
@@ -601,10 +604,13 @@
virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virSecurityLabelDefPtr secdef;
int ret = -1;
-if (!priv-dynamicOwnership)
-return 0;
+secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+
+if (!priv-dynamicOwnership || (secdef secdef-norelabel))
+ return 0;
if (dev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
return 0;
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 0/7] Honor DAC norelabel attribute
V3 of Michal's series to honor relabel='no' in device config https://www.redhat.com/archives/libvir-list/2014-April/msg00196.html In V3, the patches have been further split to ease review as requested by Jan Tomko. Jim Fehlig (7): security_dac: annotate some functions with ATTRIBUTE_NONNULL security_dac: cleanup use of enum types security_dac: rework callback parameter passing security_dac: avoid relabeling when relabel='no' security_dac: honor relabel='no' in disk config security_dac: avoid relabeling hostdevs when relabel='no' security_dac: honor relabel='no' in chardev config src/security/security_dac.c | 333 +++- 1 file changed, 203 insertions(+), 130 deletions(-) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 7/7] security_dac: honor relabel='no' in chardev config
The DAC driver ignores the relabel='no' attribute in chardev config
serial type='file'
source path='/tmp/jim/test.file'
seclabel model='dac' relabel='no'/
/source
target port='0'/
/serial
This patch avoids labeling chardevs when relabel='no' is specified.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/security/security_dac.c | 65 -
1 file changed, 46 insertions(+), 19 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 4434cd0..20f349f 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -693,11 +693,13 @@
virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,
static int
virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
- virDomainChrSourceDefPtr dev)
+ virDomainChrDefPtr dev,
+ virDomainChrSourceDefPtr dev_source)
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityLabelDefPtr seclabel;
+virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
char *in = NULL, *out = NULL;
int ret = -1;
uid_t user;
@@ -705,25 +707,35 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (virSecurityDACGetIds(seclabel, priv, user, group, NULL, NULL))
-return -1;
+if (dev)
+chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
+ SECURITY_DAC_NAME);
-switch ((enum virDomainChrType) dev-type) {
+if (chr_seclabel chr_seclabel-label) {
+if (virParseOwnershipIds(chr_seclabel-label, user, group) 0)
+return -1;
+} else {
+if (virSecurityDACGetIds(seclabel, priv, user, group, NULL, NULL)
0)
+return -1;
+}
+
+switch ((enum virDomainChrType) dev_source-type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
-ret = virSecurityDACSetOwnership(dev-data.file.path, user, group);
+ret = virSecurityDACSetOwnership(dev_source-data.file.path,
+ user, group);
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
-if ((virAsprintf(in, %s.in, dev-data.file.path) 0) ||
-(virAsprintf(out, %s.out, dev-data.file.path) 0))
+if ((virAsprintf(in, %s.in, dev_source-data.file.path) 0) ||
+(virAsprintf(out, %s.out, dev_source-data.file.path) 0))
goto done;
if (virFileExists(in) virFileExists(out)) {
if ((virSecurityDACSetOwnership(in, user, group) 0) ||
(virSecurityDACSetOwnership(out, user, group) 0)) {
goto done;
}
-} else if (virSecurityDACSetOwnership(dev-data.file.path,
+} else if (virSecurityDACSetOwnership(dev_source-data.file.path,
user, group) 0) {
goto done;
}
@@ -753,27 +765,40 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
static int
virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
- virDomainChrSourceDefPtr dev)
+ virDomainDefPtr def,
+ virDomainChrDefPtr dev,
+ virDomainChrSourceDefPtr dev_source)
{
+virSecurityLabelDefPtr seclabel;
+virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
char *in = NULL, *out = NULL;
int ret = -1;
-switch ((enum virDomainChrType) dev-type) {
+seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+
+if (dev)
+chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
+ SECURITY_DAC_NAME);
+
+if (seclabel-norelabel || (chr_seclabel chr_seclabel-norelabel))
+return 0;
+
+switch ((enum virDomainChrType) dev_source-type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
-ret = virSecurityDACRestoreSecurityFileLabel(dev-data.file.path);
+ret =
virSecurityDACRestoreSecurityFileLabel(dev_source-data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
-if ((virAsprintf(out, %s.out, dev-data.file.path) 0) ||
-(virAsprintf(in, %s.in, dev-data.file.path) 0))
+if ((virAsprintf(out, %s.out, dev_source-data.file.path) 0) ||
+(virAsprintf(in, %s.in, dev_source-data.file.path) 0))
goto done;
if (virFileExists(in) virFileExists(out)) {
if ((virSecurityDACRestoreSecurityFileLabel(out) 0) ||
(virSecurityDACRestoreSecurityFileLabel(in) 0)) {
goto
[libvirt] [PATCH V3 4/7] security_dac: avoid relabeling when relabel='no'
If relabel='no' at the domain level, no need to attempt relabeling
in virSecurityDAC{Set,Restore}SecurityAllLabel().
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/security/security_dac.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 2928c1d..f46b642 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -823,12 +823,14 @@
virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr,
int migrated)
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virSecurityLabelDefPtr secdef;
size_t i;
int rc = 0;
-if (!priv-dynamicOwnership)
-return 0;
+secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+if (!priv-dynamicOwnership || (secdef secdef-norelabel))
+return 0;
VIR_DEBUG(Restoring security label on %s migrated=%d,
def-name, migrated);
@@ -898,11 +900,11 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr
mgr,
uid_t user;
gid_t group;
-if (!priv-dynamicOwnership)
-return 0;
-
secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+if (!priv-dynamicOwnership || (secdef secdef-norelabel))
+return 0;
+
for (i = 0; i def-ndisks; i++) {
/* XXX fixme - we need to recursively label the entire tree :-( */
if (virDomainDiskGetType(def-disks[i]) == VIR_STORAGE_TYPE_DIR)
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 3/7] security_dac: rework callback parameter passing
Currently, the DAC security driver passes callback data as
void params[2];
params[0] = mgr;
params[1] = def;
Clean this up by defining a structure for passing the callback
data. Moreover, there's no need to pass the whole virDomainDef
in the callback as the only thing needed in the callbacks is
virSecurityLabelDefPtr.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/security/security_dac.c | 147 ++--
1 file changed, 75 insertions(+), 72 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 28ffbdb..2928c1d 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -53,6 +53,14 @@ struct _virSecurityDACData {
char *baselabel;
};
+typedef struct _virSecurityDACCallbackData virSecurityDACCallbackData;
+typedef virSecurityDACCallbackData *virSecurityDACCallbackDataPtr;
+
+struct _virSecurityDACCallbackData {
+virSecurityManagerPtr manager;
+virSecurityLabelDefPtr secdef;
+};
+
/* returns -1 on error, 0 on success */
int
virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
@@ -82,19 +90,12 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
/* returns 1 if label isn't found, 0 on success, -1 on error */
static int
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
-virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr)
+virSecurityDACParseIds(virSecurityLabelDefPtr seclabel,
+ uid_t *uidPtr, gid_t *gidPtr)
{
-virSecurityLabelDefPtr seclabel;
-
-if (def == NULL)
+if (!seclabel || !seclabel-label)
return 1;
-seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (seclabel == NULL || seclabel-label == NULL) {
-VIR_DEBUG(DAC seclabel for domain '%s' wasn't found, def-name);
-return 1;
-}
-
if (virParseOwnershipIds(seclabel-label, uidPtr, gidPtr) 0)
return -1;
@@ -103,31 +104,24 @@ virSecurityDACParseIds(virDomainDefPtr def, uid_t
*uidPtr, gid_t *gidPtr)
static int
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
-virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
+virSecurityDACGetIds(virSecurityLabelDefPtr seclabel,
+ virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr,
gid_t **groups, int *ngroups)
{
int ret;
-if (!def !priv) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(Failed to determine default DAC seclabel
- for an unknown object));
-return -1;
-}
-
if (groups)
*groups = priv ? priv-groups : NULL;
if (ngroups)
*ngroups = priv ? priv-ngroups : 0;
-if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) = 0)
+if ((ret = virSecurityDACParseIds(seclabel, uidPtr, gidPtr)) = 0)
return ret;
if (!priv) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(DAC seclabel couldn't be determined
- for domain '%s'), def-name);
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(DAC seclabel couldn't be determined));
return -1;
}
@@ -141,20 +135,12 @@ virSecurityDACGetIds(virDomainDefPtr def,
virSecurityDACDataPtr priv,
/* returns 1 if label isn't found, 0 on success, -1 on error */
static int
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
-virSecurityDACParseImageIds(virDomainDefPtr def,
+virSecurityDACParseImageIds(virSecurityLabelDefPtr seclabel,
uid_t *uidPtr, gid_t *gidPtr)
{
-virSecurityLabelDefPtr seclabel;
-
-if (def == NULL)
+if (!seclabel || !seclabel-imagelabel)
return 1;
-seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (seclabel == NULL || seclabel-imagelabel == NULL) {
-VIR_DEBUG(DAC imagelabel for domain '%s' wasn't found, def-name);
-return 1;
-}
-
if (virParseOwnershipIds(seclabel-imagelabel, uidPtr, gidPtr) 0)
return -1;
@@ -163,25 +149,18 @@ virSecurityDACParseImageIds(virDomainDefPtr def,
static int
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
-virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
+virSecurityDACGetImageIds(virSecurityLabelDefPtr seclabel,
+ virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr)
{
int ret;
-if (!def !priv) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(Failed to determine default DAC imagelabel
- for an unknown object));
-return -1;
-}
-
-if ((ret = virSecurityDACParseImageIds(def, uidPtr, gidPtr)) = 0)
+if ((ret = virSecurityDACParseImageIds(seclabel, uidPtr, gidPtr)) = 0)
return ret;
if (!priv) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
Re: [libvirt] [PATCH 0/4] more enum cleanups
On 15.05.2014 00:45, Eric Blake wrote:
Inspired by the cleanups contributed by Julio Faracco, I did some
more cleanups of my own. My end goal is to turn on a syntax-check
rule that forbids 'enum vir' (since we should always be declaring
'typedef enum { ... } vir...;'), but there's still a lot of
violations in the code base, even after this series.
Eric Blake (4):
vbox: fix stale comment about vdi storage type
maint: use enum typedef for virstorageencryption.h
maint: shorten 'TypeType' function names
maint: prefer enum over int for virstoragefile structs
src/conf/domain_conf.c | 14 +++---
src/conf/domain_conf.h | 2 +-
src/conf/secret_conf.c | 8
src/conf/secret_conf.h | 4 ++--
src/conf/storage_conf.c| 14 +++---
src/conf/storage_conf.h| 6 +++---
src/libvirt_private.syms | 10 +-
src/lxc/lxc_controller.c | 2 +-
src/qemu/qemu_command.c| 8
src/qemu/qemu_driver.c | 8
src/storage/storage_backend_disk.c | 2 +-
src/util/virstorageencryption.c| 6 +++---
src/util/virstorageencryption.h| 10 +-
src/util/virstoragefile.h | 19 ++-
src/vbox/vbox_tmpl.c | 15 ---
tools/virsh-secret.c | 4 ++--
16 files changed, 67 insertions(+), 65 deletions(-)
ACK to all.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 1/7] security_dac: annotate some functions with ATTRIBUTE_NONNULL
Annotate some static function parameters with ATTRIBUTE_NONNULL
and remove checks for NULL inputs.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/security/security_dac.c | 34 ++
1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index ed79857..19742ed 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -81,10 +81,9 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
/* returns 1 if label isn't found, 0 on success, -1 on error */
static int
+ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr)
{
-uid_t uid;
-gid_t gid;
virSecurityLabelDefPtr seclabel;
if (def == NULL)
@@ -96,18 +95,14 @@ virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr,
gid_t *gidPtr)
return 1;
}
-if (virParseOwnershipIds(seclabel-label, uid, gid) 0)
+if (virParseOwnershipIds(seclabel-label, uidPtr, gidPtr) 0)
return -1;
-if (uidPtr)
-*uidPtr = uid;
-if (gidPtr)
-*gidPtr = gid;
-
return 0;
}
static int
+ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr,
gid_t **groups, int *ngroups)
@@ -136,10 +131,8 @@ virSecurityDACGetIds(virDomainDefPtr def,
virSecurityDACDataPtr priv,
return -1;
}
-if (uidPtr)
-*uidPtr = priv-user;
-if (gidPtr)
-*gidPtr = priv-group;
+*uidPtr = priv-user;
+*gidPtr = priv-group;
return 0;
}
@@ -147,11 +140,10 @@ virSecurityDACGetIds(virDomainDefPtr def,
virSecurityDACDataPtr priv,
/* returns 1 if label isn't found, 0 on success, -1 on error */
static int
+ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
virSecurityDACParseImageIds(virDomainDefPtr def,
uid_t *uidPtr, gid_t *gidPtr)
{
-uid_t uid;
-gid_t gid;
virSecurityLabelDefPtr seclabel;
if (def == NULL)
@@ -163,18 +155,14 @@ virSecurityDACParseImageIds(virDomainDefPtr def,
return 1;
}
-if (virParseOwnershipIds(seclabel-imagelabel, uid, gid) 0)
+if (virParseOwnershipIds(seclabel-imagelabel, uidPtr, gidPtr) 0)
return -1;
-if (uidPtr)
-*uidPtr = uid;
-if (gidPtr)
-*gidPtr = gid;
-
return 0;
}
static int
+ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr)
{
@@ -197,10 +185,8 @@ virSecurityDACGetImageIds(virDomainDefPtr def,
virSecurityDACDataPtr priv,
return -1;
}
-if (uidPtr)
-*uidPtr = priv-user;
-if (gidPtr)
-*gidPtr = priv-group;
+*uidPtr = priv-user;
+*gidPtr = priv-group;
return 0;
}
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 2/7] security_dac: cleanup use of enum types
In switch statements, use enum types since it is safer when
adding new items to the enum.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
Hmm, may conflict with some of the work I've seen lately to cleanup enum
declarations throughout the code.
src/security/security_dac.c | 41 +++--
1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 19742ed..28ffbdb 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -472,7 +472,7 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr
mgr,
if (dev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
return 0;
-switch (dev-source.subsys.type) {
+switch ((enum virDomainHostdevSubsysType) dev-source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
virUSBDevicePtr usb;
@@ -540,7 +540,7 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr
mgr,
break;
}
-default:
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
ret = 0;
break;
}
@@ -593,7 +593,7 @@
virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,
if (dev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
return 0;
-switch (dev-source.subsys.type) {
+switch ((enum virDomainHostdevSubsysType) dev-source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
virUSBDevicePtr usb;
@@ -658,7 +658,7 @@
virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,
break;
}
-default:
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
ret = 0;
break;
}
@@ -683,7 +683,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
if (virSecurityDACGetIds(def, priv, user, group, NULL, NULL))
return -1;
-switch (dev-type) {
+switch ((enum virDomainChrType) dev-type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
ret = virSecurityDACSetOwnership(dev-data.file.path, user, group);
@@ -705,7 +705,17 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
ret = 0;
break;
-default:
+case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+case VIR_DOMAIN_CHR_TYPE_NULL:
+case VIR_DOMAIN_CHR_TYPE_VC:
+case VIR_DOMAIN_CHR_TYPE_PTY:
+case VIR_DOMAIN_CHR_TYPE_STDIO:
+case VIR_DOMAIN_CHR_TYPE_UDP:
+case VIR_DOMAIN_CHR_TYPE_TCP:
+case VIR_DOMAIN_CHR_TYPE_UNIX:
+case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+case VIR_DOMAIN_CHR_TYPE_NMDM:
+case VIR_DOMAIN_CHR_TYPE_LAST:
ret = 0;
break;
}
@@ -723,7 +733,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
char *in = NULL, *out = NULL;
int ret = -1;
-switch (dev-type) {
+switch ((enum virDomainChrType) dev-type) {
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_FILE:
ret = virSecurityDACRestoreSecurityFileLabel(dev-data.file.path);
@@ -744,7 +754,17 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr
mgr ATTRIBUTE_UNUSED,
ret = 0;
break;
-default:
+case VIR_DOMAIN_CHR_TYPE_NULL:
+case VIR_DOMAIN_CHR_TYPE_VC:
+case VIR_DOMAIN_CHR_TYPE_PTY:
+case VIR_DOMAIN_CHR_TYPE_STDIO:
+case VIR_DOMAIN_CHR_TYPE_UDP:
+case VIR_DOMAIN_CHR_TYPE_TCP:
+case VIR_DOMAIN_CHR_TYPE_UNIX:
+case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+case VIR_DOMAIN_CHR_TYPE_NMDM:
+case VIR_DOMAIN_CHR_TYPE_LAST:
ret = 0;
break;
}
@@ -1047,7 +1067,7 @@ virSecurityDACGenLabel(virSecurityManagerPtr mgr,
return rc;
}
-switch (seclabel-type) {
+switch ((virDomainSeclabelType) seclabel-type) {
case VIR_DOMAIN_SECLABEL_STATIC:
if (seclabel-label == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1071,7 +1091,8 @@ virSecurityDACGenLabel(virSecurityManagerPtr mgr,
case VIR_DOMAIN_SECLABEL_NONE:
/* no op */
return 0;
-default:
+case VIR_DOMAIN_SECLABEL_DEFAULT:
+case VIR_DOMAIN_SECLABEL_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_(unexpected security label type '%s'),
virDomainSeclabelTypeToString(seclabel-type));
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 5/7] security_dac: honor relabel='no' in disk config
https://bugzilla.redhat.com/show_bug.cgi?id=999301
The DAC driver ignores the relabel='no' attribute in disk config
disk type='file' device='floppy'
driver name='qemu' type='raw'/
source file='/some/path/floppy.img'
seclabel model='dac' relabel='no'/
/source
target dev='fda' bus='fdc'/
readonly/
/disk
This patch avoid labeling disks when relabel='no' is specified.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/security/security_dac.c | 36
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index f46b642..d6ca303 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -289,7 +289,7 @@ virSecurityDACRestoreSecurityFileLabel(const char *path)
static int
-virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
+virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk,
const char *path,
size_t depth ATTRIBUTE_UNUSED,
void *opaque)
@@ -298,11 +298,23 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr
disk ATTRIBUTE_UNUSED,
virSecurityManagerPtr mgr = cbdata-manager;
virSecurityLabelDefPtr secdef = cbdata-secdef;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virSecurityDeviceLabelDefPtr disk_seclabel;
uid_t user;
gid_t group;
-if (virSecurityDACGetImageIds(secdef, priv, user, group))
-return -1;
+disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk,
+SECURITY_DAC_NAME);
+
+if (disk_seclabel disk_seclabel-norelabel)
+return 0;
+
+if (disk_seclabel disk_seclabel-label) {
+if (virParseOwnershipIds(disk_seclabel-label, user, group) 0)
+return -1;
+} else {
+if (virSecurityDACGetImageIds(secdef, priv, user, group))
+return -1;
+}
return virSecurityDACSetOwnership(path, user, group);
}
@@ -326,6 +338,9 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr
mgr,
secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+if (secdef secdef-norelabel)
+return 0;
+
cbdata.manager = mgr;
cbdata.secdef = secdef;
return virDomainDiskDefForeachPath(disk,
@@ -337,11 +352,13 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr
mgr,
static int
virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
- virDomainDefPtr def
ATTRIBUTE_UNUSED,
+ virDomainDefPtr def,
virDomainDiskDefPtr disk,
int migrated)
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virSecurityLabelDefPtr secdef;
+virSecurityDeviceLabelDefPtr disk_seclabel;
const char *src = virDomainDiskGetSource(disk);
if (!priv-dynamicOwnership)
@@ -350,6 +367,17 @@
virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK)
return 0;
+secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+
+if (secdef secdef-norelabel)
+return 0;
+
+disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk,
+SECURITY_DAC_NAME);
+
+if (disk_seclabel disk_seclabel-norelabel)
+return 0;
+
/* Don't restore labels on readoly/shared disks, because
* other VMs may still be accessing these
* Alternatively we could iterate over all running
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/2] security_dac: Honor norelabel attribute
Ján Tomko wrote: On 04/04/2014 02:34 PM, Michal Privoznik wrote: [...] src/security/security_dac.c | 92 +++-- 1 file changed, 73 insertions(+), 19 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 8835d49..f15a0e9 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -286,7 +286,7 @@ virSecurityDACRestoreSecurityFileLabel(const char *path) static int -virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, +virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk, const char *path, size_t depth ATTRIBUTE_UNUSED, void *opaque) @@ -295,11 +295,23 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, virSecurityManagerPtr mgr = cbdata-manager; virSecurityLabelDefPtr secdef = cbdata-secdef; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); +virSecurityDeviceLabelDefPtr disk_seclabel; uid_t user; gid_t group; -if (virSecurityDACGetImageIds(secdef, priv, user, group) 0) -return -1; +disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk, +SECURITY_DAC_NAME); + +if (disk_seclabel disk_seclabel-norelabel) +return 0; What if the domain label has relabel='no', but the disk label has relabel='yes'? Seems that configuration is not valid. When trying it, I get error: XML error: label overrides require relabeling to be enabled at the domain level Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/1] migration: add support for migrateURI configuration
Hi,
Firstly thanks for your attention.
On Thu, 2014-05-15 at 11:50 +0200, Jiri Denemark wrote:
On Wed, May 14, 2014 at 15:18:09 +0800, Chen Fan wrote:
For now, we set the migration URI via command line '--migrate_uri' or
construct the URI by looking up the dest host's hostname which could be
solved by DNS automatically.
But in cases the dest host have two or more NICs to reach, we may need to
send the migration data over a specific NIC which is different from the
automatically resloved one for some reason like performance, security, etc.
thus we must explicitly specify the migrateuri in command line everytime,
but it is too troublesome if there are many such hosts(and don't forget
virt-manager).
This patches add a configuration file option on dest host to save the
default migrate uri which explicitly specify which of this host's
addresses is used for transferring data, thus user doesn't boring
to specify it in command line everytime.
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
---
v3-v4: move up the default uri_in setting to
qemuDomainMigratePrepare3Params()
src/qemu/qemu.conf | 6 +-
src/qemu/qemu_conf.c | 1 +
src/qemu/qemu_conf.h | 1 +
src/qemu/qemu_driver.c | 2 +-
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index f0e802f..6b443d0 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -449,7 +449,11 @@
#
#seccomp_sandbox = 1
-
+# Override the migration URI for specifying one of host's IP addresses
+# to transfer the migration data stream.
+# Defaults to hostname, both IPv4 and IPv6 addresses are accepted.
+#
+#migrate_uri = tcp://192.168.0.1
# Override the listen address for all incoming migrations. Defaults to
# 0.0.0.0, or :: if both host and qemu are capable of IPv6.
The more I think about this the more I incline to a slightly different
approach. Rather than providing a way to override migration URI, we
could just provide an option in libvirtd.conf to override what
virGetHostname returns. That is, the option (naturally called hostname)
would tell libvirt to use the configured hostname (which might even be
just an IP address) instead of trying to detect it.
If I understand correctly. you prefer using a hostname(or IP address)
option in libvirtd.conf to configuring a default migration URI in
qemu.conf, right? do you want to affect the all virGetHostname()
returns? I'm afraid I can't see any benefit to the goal, could you tell
me that?
Thanks,
Chen
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 198ee2f..43361dc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -574,6 +574,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr
cfg,
GET_VALUE_LONG(seccomp_sandbox, cfg-seccompSandbox);
+GET_VALUE_STR(migrate_uri, cfg-migrateUri);
GET_VALUE_STR(migration_address, cfg-migrationAddress);
ret = 0;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a36ea63..f99c56e 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -163,6 +163,7 @@ struct _virQEMUDriverConfig {
int seccompSandbox;
+char *migrateUri;
/* The default for -incoming */
char *migrationAddress;
int migrationPortMin;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fca1a91..56c24b5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10888,7 +10888,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
virDomainDefPtr def = NULL;
const char *dom_xml = NULL;
const char *dname = NULL;
-const char *uri_in = NULL;
+const char *uri_in = cfg-migrateUri;
const char *listenAddress = cfg-migrationAddress;
char *origname = NULL;
int ret = -1;
And in any case, the change you made between v3 and v4 is wrong, since
now you are only change one entry point to the Prepare phase while
changing qemuMigrationPrepareDirect makes this work for all APIs and
migration protocol versions.
Oh, you are right. thanks.
Chen
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
