The patch
http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=de0aeafe9ce3eb414c8b5d3aa8995d776a2952de
removes invtsc flag in the host-model CPU.
I'm wondering, will it be better to pass args "migratable=no/yes" to qemu, and
let qemu complete the
remaining work? As that qemu has checked whether it's necessary to use invtsc
or not.
--
"invtsc is available only if using: -cpu host,migratable=no,+invtsc."
http://git.qemu.org/?p=qemu.git;a=commit;h=120eee7d1fdb2eba15766cfff7b9bcdc902690b4
--
There's another problem, if we do not pass "migratable=no" to qemu.
Consider if we set host mode to pass-through
-
-
then the vm->def->cpu->features contains invtsc. however, qemu will
automatically remove this cpu flag
as that "migration=no" is not passed to it. thus, the guest will not start up.
This problem is in fact
caused by the patch:
http://libvirt.org/git/?p=libvirt.git;a=commit;h=fba6bc47cbcabbe08d42279691efb0dff3b9c997,
it forbids guest domain to start up if the host has INVTSC while the
guest(qemu) does not.
-
for (i = 0; def->cpu && 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;
--
In conclusion:
1 Will it better to pass args "migratable=yes/no" to qemu rather than doing the
mask-invtsc job in libvirt?
2 If the guest has "pass-through" cpu mode, then it's unable to start up,
because qemu removes invtsc, and
vm->def->cpu->features has it. It seems a BUG.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
> -Original Message-
> From: Michal Privoznik [mailto:mpriv...@redhat.com]
>
> On 23.09.2014 05:40, Chen Hanxiao wrote:
> > We don't accept any other values except '0'.
> >
> > Signed-off-by: Chen Hanxiao
> > ---
> > docs/formatdomain.html.in | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index eefdd5e..bb72452 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -315,7 +315,7 @@
> >
> >
> > start
> > - First user ID in container.
> > + First user ID in container. It must be '0'.
> > target
> > The first user ID in container will be mapped to this target
> > user
> > ID in host.
> >
>
> This is not entirely true. Only the first id mapping must refer to root.
> And by first I mean in a sorted array of mappings. For instance:
>
>
>
>
>
>
>
>
> is okay.
I missed that part.
Thanks for clarification.
- Chen
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/23/2014 03:26 PM, Pavel Hrdina wrote:
>>> +
>>> +VIR_DEBUG("Relaying domain tunable event %s %d, callback %d",
>>> + dom->name, dom->id, callback->callbackID);
>>> +
>>
>> Might also be nice to log "%p %n", params, nparams
>
> Yes, that would be probably nice, but since I've pushed this patch
> already I can create a following patch with this small update?
>
Yes, a followup is fine.
>>
>>
>>> +++ b/src/remote/remote_protocol.x
>>> @@ -247,6 +247,9 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;
>>> /* Upper limit on count of parameters returned via bulk stats API */
>>> const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;
>>>
>>> +/* Upper limit of message size for tunable event. */
>>> +const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 8388608;
>>
>> That feels excessive...
>>
>>> +
>>> /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */
>>> typedef opaque remote_uuid[VIR_UUID_BUFLEN];
>>>
>>> @@ -2990,6 +2993,12 @@ struct remote_domain_event_block_job_2_msg {
>>> int status;
>>> };
>>>
>>> +struct remote_domain_event_callback_tunable_msg {
>>> +int callbackID;
>>> +remote_nonnull_domain dom;
>>> +remote_typed_param params;
>>
>> ...each param in the array will occupy multiple bytes. I think that
>> something as low as 2048 for REMOTE_DOMAIN_EVENT_TUNABLE_MAX is still
>> plenty (we don't have that many tunables yet); even if each tunable
>> requires 64 bytes to transmit (mostly in the name of the parameter, but
>> also in the type and value), that's still well under a megabyte limit of
>> information passed on an instance of the event.
>>
>
> Well, yes and no :). Let's say, that we will add in the future (and I'm
> planning to do it) blkiotune where you can update at the same time all
> of the tunables for all host's disks where all params for now will be
> only VIR_TYPED_PARAM_STRING and that could consume a lot of memory. I
> know that it will probably never be that much, but I wanted to be sure
> that we will have enough space for all possible tunable events.
Still, are you going to return 8 million separate strings? Or just 8
million bytes but still contained within 2000 strings? Seriously, I
think 2048 is a perfectly LARGE limit - there are not THAT many tunables
per domain. The params is not the overall size of the command,
but the number of parameters (each of which can be quite large if they
are type string)
--
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
On 09/23/2014 10:08 PM, Eric Blake wrote:
On 09/23/2014 12:46 PM, Pavel Hrdina wrote:
This new event will use typedParameters to expose what has been actually
updated and the reason is that we can in the future extend any tunable
values or add new tunable values. With typedParameters we don't have to
worry about creating some other events, we will just use this universal
event to inform user about updates.
Signed-off-by: Pavel Hrdina
---
When the event is issued, does it contain ONLY parameters for what just
changed, or does it list ALL tunables including the unchanged ones?
It feels like your API is only capable of listing the new tunable value.
Is anyone using this event going to want to know both former and new
value at the same time?
The API returns only the new tunable values. I've inspired myself with
other APIs that they also returns only the updated values without the
old ones.
The only user that I know right now will be oVirt and they seems to be
care only about new values.
+/**
+ * virConnectDomainEventTunableCallback:
+ * @conn: connection object
+ * @dom: domain on which the event occurred
+ * @params: changed tunable values stored as array of virTypedParameter
+ * @nparams: size of the array
+ * @opaque: application specified data
+ *
+ * This callback occurs when tunable values are updated. The params must not
+ * be freed in the callback handler as it's done internally after the callback
+ * handler is executed.
+ *
+ * The callback signature to use when registering for an event of type
+ * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny()
+ */
+typedef void (*virConnectDomainEventTunableCallback)(virConnectPtr conn,
+ virDomainPtr dom,
+ virTypedParameterPtr
params,
+ int nparams,
+ void *opaque);
Where do we document what names of tunables to expect?
It's documented in the 4/4 patch as I'm adding for now only cputune and
it will be documented in the description of this callback as existing
namespace and also there will be definitions for each tunable value that
we will return.
+++ b/daemon/remote.c
+static int
+remoteRelayDomainEventTunable(virConnectPtr conn,
+ virDomainPtr dom,
+ virTypedParameterPtr params,
+ int nparams,
+ void *opaque)
+{
+daemonClientEventCallbackPtr callback = opaque;
+remote_domain_event_callback_tunable_msg data;
+
+if (callback->callbackID < 0 ||
+!remoteRelayDomainEventCheckACL(callback->client, conn, dom))
+return -1;
+
+VIR_DEBUG("Relaying domain tunable event %s %d, callback %d",
+ dom->name, dom->id, callback->callbackID);
+
Might also be nice to log "%p %n", params, nparams
Yes, that would be probably nice, but since I've pushed this patch
already I can create a following patch with this small update?
+++ b/src/remote/remote_protocol.x
@@ -247,6 +247,9 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;
/* Upper limit on count of parameters returned via bulk stats API */
const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;
+/* Upper limit of message size for tunable event. */
+const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 8388608;
That feels excessive...
+
/* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */
typedef opaque remote_uuid[VIR_UUID_BUFLEN];
@@ -2990,6 +2993,12 @@ struct remote_domain_event_block_job_2_msg {
int status;
};
+struct remote_domain_event_callback_tunable_msg {
+int callbackID;
+remote_nonnull_domain dom;
+remote_typed_param params;
...each param in the array will occupy multiple bytes. I think that
something as low as 2048 for REMOTE_DOMAIN_EVENT_TUNABLE_MAX is still
plenty (we don't have that many tunables yet); even if each tunable
requires 64 bytes to transmit (mostly in the name of the parameter, but
also in the type and value), that's still well under a megabyte limit of
information passed on an instance of the event.
Well, yes and no :). Let's say, that we will add in the future (and I'm
planning to do it) blkiotune where you can update at the same time all
of the tunables for all host's disks where all params for now will be
only VIR_TYPED_PARAM_STRING and that could consume a lot of memory. I
know that it will probably never be that much, but I wanted to be sure
that we will have enough space for all possible tunable events.
Pavel
The code looks okay, but I'm still a bit worried about the design.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Eric Blake wrote:
> On 09/23/2014 11:53 AM, Jim Fehlig wrote:
>
>> When libvirt-guests is configured to start guests on host
>> boot, it is possible for guests start and read the host
>> clock before it is synchronized. Services such as
>> libvirt-guests that require correct time should use the
>> Special Passive System Unit time-sync.target
>>
>> http://www.freedesktop.org/software/systemd/man/systemd.special.html#time-sync.target
>> Signed-off-by: Jim Fehlig
>> ---
>>
>> V2 of
>>
>> https://www.redhat.com/archives/libvir-list/2014-September/msg00426.html
>>
>> In V2: Use time-sync.target instead of ntp-wait.service
>>
>
> ACK
>
Thanks, pushed now.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/18/2014 05:54 AM, Peter Krempa wrote:
> Report also the name of the parent file and uid/gid used to access it to
> help debugging broken storage configurations.
> ---
> src/storage/storage_driver.c | 20 +++-
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
ACK
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/23/2014 12:46 PM, Pavel Hrdina wrote:
> This new event will use typedParameters to expose what has been actually
> updated and the reason is that we can in the future extend any tunable
> values or add new tunable values. With typedParameters we don't have to
> worry about creating some other events, we will just use this universal
> event to inform user about updates.
>
> Signed-off-by: Pavel Hrdina
> ---
>
When the event is issued, does it contain ONLY parameters for what just
changed, or does it list ALL tunables including the unchanged ones?
It feels like your API is only capable of listing the new tunable value.
Is anyone using this event going to want to know both former and new
value at the same time?
>
> +/**
> + * virConnectDomainEventTunableCallback:
> + * @conn: connection object
> + * @dom: domain on which the event occurred
> + * @params: changed tunable values stored as array of virTypedParameter
> + * @nparams: size of the array
> + * @opaque: application specified data
> + *
> + * This callback occurs when tunable values are updated. The params must not
> + * be freed in the callback handler as it's done internally after the
> callback
> + * handler is executed.
> + *
> + * The callback signature to use when registering for an event of type
> + * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny()
> + */
> +typedef void (*virConnectDomainEventTunableCallback)(virConnectPtr conn,
> + virDomainPtr dom,
> + virTypedParameterPtr
> params,
> + int nparams,
> + void *opaque);
Where do we document what names of tunables to expect?
> +++ b/daemon/remote.c
> +static int
> +remoteRelayDomainEventTunable(virConnectPtr conn,
> + virDomainPtr dom,
> + virTypedParameterPtr params,
> + int nparams,
> + void *opaque)
> +{
> +daemonClientEventCallbackPtr callback = opaque;
> +remote_domain_event_callback_tunable_msg data;
> +
> +if (callback->callbackID < 0 ||
> +!remoteRelayDomainEventCheckACL(callback->client, conn, dom))
> +return -1;
> +
> +VIR_DEBUG("Relaying domain tunable event %s %d, callback %d",
> + dom->name, dom->id, callback->callbackID);
> +
Might also be nice to log "%p %n", params, nparams
> +++ b/src/remote/remote_protocol.x
> @@ -247,6 +247,9 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;
> /* Upper limit on count of parameters returned via bulk stats API */
> const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;
>
> +/* Upper limit of message size for tunable event. */
> +const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 8388608;
That feels excessive...
> +
> /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */
> typedef opaque remote_uuid[VIR_UUID_BUFLEN];
>
> @@ -2990,6 +2993,12 @@ struct remote_domain_event_block_job_2_msg {
> int status;
> };
>
> +struct remote_domain_event_callback_tunable_msg {
> +int callbackID;
> +remote_nonnull_domain dom;
> +remote_typed_param params;
...each param in the array will occupy multiple bytes. I think that
something as low as 2048 for REMOTE_DOMAIN_EVENT_TUNABLE_MAX is still
plenty (we don't have that many tunables yet); even if each tunable
requires 64 bytes to transmit (mostly in the name of the parameter, but
also in the type and value), that's still well under a megabyte limit of
information passed on an instance of the event.
The code looks okay, but I'm still a bit worried about the design.
--
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
On 09/23/2014 09:37 PM, John Ferlan wrote:
On 09/23/2014 02:47 PM, Pavel Hrdina wrote:
Now we have universal tunable event so we can use it for reporting
changes to user. The cputune values will be prefixed with "cputune" to
distinguish it from other tunable events.
Signed-off-by: Pavel Hrdina
---
since v4:
- added macro definitions for cputune typedParameters fileds
include/libvirt/libvirt.h.in | 63 ++
src/qemu/qemu_cgroup.c | 19 ++-
src/qemu/qemu_driver.c | 81
3 files changed, 162 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 86be86f..898f8b5 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5204,6 +5204,66 @@ typedef void
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
void *opaque);
/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN:
+ *
+ * Macro represents formatted pinning for one vcpu specified by id which is
+ * appended to the parameter name, for example "cputune.vcpupin1",
+ * as VIR_TYPED_PARAM_STRING.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN "cputune.vcpupin%u"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN:
+ *
+ * Macro represents formatted pinning for emulator process,
+ * as VIR_TYPED_PARAM_STRING.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN "cputune.emulatorpin"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES:
+ *
+ * Macro represents proportional weight of the scheduler used on the
+ * host cpu, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES "cputune.cpu_shares"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD:
+ *
+ * Macro represents the enforcement period for a quota, in microseconds,
+ * for vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD "cputune.vcpu_period"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA:
+ *
+ * Macro represents the maximum bandwidth to be used within a period for
+ * vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_LLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA "cputune.vcpu_quota"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD:
+ *
+ * Macro represents the enforcement period for a quota in microseconds,
+ * when using the posix scheduler, for all emulator activity not tied to
+ * vcpus, as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD "cputune.emulator_period"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA:
+ *
+ * Macro represents the maximum bandwidth to be used within a period for
+ * all emulator activity not tied to vcpus, when using the posix scheduler,
+ * as an VIR_TYPED_PARAM_LLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA "cputune.emulator_quota"
+
+
+/**
* virConnectDomainEventTunableCallback:
* @conn: connection object
* @dom: domain on which the event occurred
@@ -5215,6 +5275,9 @@ typedef void
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
* be freed in the callback handler as it's done internally after the callback
* handler is executed.
*
+ * Currently supported name spaces:
+ * "cputune.*"
+ *
* The callback signature to use when registering for an event of type
* VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny()
*/
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 7c6b2c1..41d7057 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -34,6 +34,7 @@
#include "virscsi.h"
#include "virstring.h"
#include "virfile.h"
+#include "virtypedparam.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -676,6 +677,10 @@ static int
qemuSetupCpuCgroup(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
+virObjectEventPtr event = NULL;
+virTypedParameterPtr eventParams = 0;
s/0/NULL/
ACK w/ that.
And again thanks, pushed.
Pavel
John
+int eventNparams = 0;
+int eventMaxparams = 0;
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
if (vm->def->cputune.sharesSpecified) {
@@ -694,7 +699,19 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
if (virCgroupGetCpuShares(priv->cgroup, &val) < 0)
return -1;
-vm->def->cputune.shares = val;
+if (vm->def->cputune.shares != val) {
+vm->def->cputune.shares = val;
+if (virTypedParamsAddULLong(&eventParams, &eventNparams,
+&eventMaxparams,
+VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES,
+val) < 0)
+return -1;
+
+event = virDomainEventTunableNewFromObj(vm, eventParams,
eventNparams);
+}
+
+if (event)
+qemuDomainEventQueue(vm->pr
On 09/23/2014 09:36 PM, John Ferlan wrote:
On 09/23/2014 02:46 PM, Pavel Hrdina wrote:
This new event will use typedParameters to expose what has been actually
updated and the reason is that we can in the future extend any tunable
values or add new tunable values. With typedParameters we don't have to
worry about creating some other events, we will just use this universal
event to inform user about updates.
Signed-off-by: Pavel Hrdina
---
since v4:
- changed cpu-tune to tunable in virsh-domain.c
- added REMOTE_DOMAIN_EVENT_TUNABLE_MAX to limit maximal length of tunable
event msg
daemon/remote.c | 45 +
include/libvirt/libvirt.h.in | 22 +++
src/conf/domain_event.c | 93
src/conf/domain_event.h | 9 +
src/libvirt_private.syms | 2 +
src/remote/remote_driver.c | 42
src/remote/remote_protocol.x | 17 +++-
src/remote_protocol-structs | 9 +
tools/virsh-domain.c | 33
9 files changed, 271 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index daa4b60..ddd510c 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -111,6 +111,13 @@ remoteDeserializeTypedParameters(remote_typed_param
*args_params_val,
int *nparams);
static int
+remoteSerializeTypedParameters(virTypedParameterPtr params,
+ int nparams,
+ remote_typed_param **ret_params_val,
+ u_int *ret_params_len,
+ unsigned int flags);
+
+static int
remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors,
int nerrors,
remote_domain_disk_error **ret_errors_val,
@@ -969,6 +976,43 @@ remoteRelayDomainEventBlockJob2(virConnectPtr conn,
}
+static int
+remoteRelayDomainEventTunable(virConnectPtr conn,
+ virDomainPtr dom,
+ virTypedParameterPtr params,
+ int nparams,
+ void *opaque)
+{
+daemonClientEventCallbackPtr callback = opaque;
+remote_domain_event_callback_tunable_msg data;
+
+if (callback->callbackID < 0 ||
+!remoteRelayDomainEventCheckACL(callback->client, conn, dom))
+return -1;
+
+VIR_DEBUG("Relaying domain tunable event %s %d, callback %d",
+ dom->name, dom->id, callback->callbackID);
+
+/* build return data */
+memset(&data, 0, sizeof(data));
+data.callbackID = callback->callbackID;
+make_nonnull_domain(&data.dom, dom);
+
+if (remoteSerializeTypedParameters(params, nparams,
+ &data.params.params_val,
+ &data.params.params_len,
+ VIR_TYPED_PARAM_STRING_OKAY) < 0)
+return -1;
+
+remoteDispatchObjectEventSend(callback->client, remoteProgram,
+ REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE,
+
(xdrproc_t)xdr_remote_domain_event_callback_tunable_msg,
+ &data);
+
+return 0;
+}
+
+
static virConnectDomainEventGenericCallback domainEventCallbacks[] = {
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot),
@@ -987,6 +1031,7 @@ static virConnectDomainEventGenericCallback
domainEventCallbacks[] = {
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventPMSuspendDisk),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceRemoved),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockJob2),
+VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable),
};
verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST);
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 6371b7b..86be86f 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5203,6 +5203,27 @@ typedef void
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
const char
*devAlias,
void *opaque);
+/**
+ * virConnectDomainEventTunableCallback:
+ * @conn: connection object
+ * @dom: domain on which the event occurred
+ * @params: changed tunable values stored as array of virTypedParameter
+ * @nparams: size of the array
+ * @opaque: application specified data
+ *
+ * This callback occurs when tunable values are updated. The params must not
+ * be freed in the callback handler as it's done internally after the callback
+ * handler is executed.
+ *
+ * The callback signature to use when registering for an event of type
+ * VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEve
On 09/18/2014 02:24 AM, Michal Privoznik wrote:
> A long time ago in a galaxy far, far away it has been decided
> that libvirt will manage not only domains but host as well. And
> with my latest work on qemu driver supporting huge pages, we miss
> the cherry on top: an API to allocate huge pages on the run.
> Currently users are forced to log into the host and adjust the
> huge pages pool themselves. However, with this API the problem
> is gone - they can both size up and size down the pool.
>
> Signed-off-by: Michal Privoznik
> ---
> +/**
> + * virNodeAllocPages:
> + * @conn: pointer to the hypervisor connection
> + * @npages: number of items in the @pageSizes array
I'd mention '@pageSizes and @pageCounts arrays'
> + * @pageSizes: which huge page sizes to allocate
> + * @pageCounts: how many pages should be allocated
> + * @startCell: index of first cell to allocate pages on
> + * @cellCount: number of consecutive cells to allocate pages on
> + * @flags: extra flags; binary-OR of virConnectGetAllDomainStatsFlags
> + *
> + * Sometimes, when trying to start a new domain, it may be
> + * necessary to reserve some huge pages in the system pool which
> + * can be then allocated by the domain. This API serves that
> + * purpose. On its input, @pageSizes and @pageCounts are arrays
> + * of the same cardinality of @npages. The @pageSizes contains
> + * page sizes which are to be allocated in the system (the size
> + * unit is kibibytes), and @pageCounts then contains the number
> + * of pages to reserve. Depending on @flags specified, the number
> + * of pages is either added into the pool and the pool is sized
> + * up (@flags have VIR_NODE_ALLOC_PAGES_ADD set) or the number of
Technically, VIR_NODE_ALLOC_PAGES_ADD is 0, so it can't be set :) Do we
even need this named constant, or can we just drop it?
> + * pages is looked at the new size of pool and the pool can be
> + * both sized up or down (@flags have VIR_NODE_ALLOC_PAGES_SET
The wording is a bit awkward; maybe:
If @flags is 0 (VIR_NODE_ALLOC_PAGES_ADD), each pool corresponding to
@pageSizes grows by the number of pages specified in the corresponding
@pageCounts. If @flags contains VIR_NODE_ALLOC_PAGES_SET, each pool
mentioned is resized to the given number of pages.
> + * set). The pages pool can be allocated over several NUMA nodes
> + * at once, just point at @startCell and tell how many subsequent
> + * NUMA nodes should be taken in.
Is there shorthand such as @startCell==-1 for allocating across all NUMA
nodes? Is there checking that @npages cannot exceed the number of NUMA
nodes available starting at @startCell through @cellCount?
If I understand correctly, usage is something like (pseudo-code):
virNodeAllocPages(conn, 2, (int[]){ [0]=2M, [1]=1G },
(int[]){ [0]=1024, [1]=1 },
1, 2, 0)
which proceeds to add 1024 pages of size 2M, and 1 page of size 1G, to
the pools associated both with NUMA node 1 and NUMA node 2 (that is, I
just allocated 6G across two nodes).
> + *
> + * Returns: the number of nodes successfully adjusted or -1 in
> + * case of an error.
Can this function partially succeed? That is, if I pass @npages==2 and
@cellCount==2, will I ever get a non-negative result less than 4? If
the result is 2, which allocations failed (are all allocations on the
first cell attempted before any on the second, or are all allocations to
the first pool size attempted across multiple cells before revisiting
cells to allocate in the second pool size)?
> + */
> +int
> +virNodeAllocPages(virConnectPtr conn,
> + unsigned int npages,
> + unsigned int *pageSizes,
> + unsigned long long *pageCounts,
> + int startCell,
> + unsigned int cellCount,
> + unsigned int flags)
> +{
> +VIR_DEBUG("conn=%p npages=%u pageSizes=%p pageCounts=%p "
> + "startCell=%d cellCount=%u flagx=%x",
> + conn, npages, pageSizes, pageCounts, startCell,
> + cellCount, flags);
> +
> +virResetLastError();
> +
> +virCheckConnectReturn(conn, -1);
> +virCheckNonZeroArgGoto(npages, error);
> +virCheckNonNullArgGoto(pageSizes, error);
> +virCheckNonNullArgGoto(pageCounts, error);
> +
Where is the validation that startCell and cellCount make sense? I'd
assume we want to ensure cellCount is positive? Or is there a special
case of a count of 0 for visiting all NUMA cells without knowing in
advance how many there are?
> +++ b/src/libvirt_public.syms
> @@ -677,6 +677,7 @@ LIBVIRT_1.2.8 {
> virDomainListGetStats;
> virDomainOpenGraphicsFD;
> virDomainStatsRecordListFree;
> +virNodeAllocPages;
> } LIBVIRT_1.2.7;
Need a new section for 1.2.9.
--
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@redha
On 09/23/2014 11:53 AM, Jim Fehlig wrote:
> When libvirt-guests is configured to start guests on host
> boot, it is possible for guests start and read the host
> clock before it is synchronized. Services such as
> libvirt-guests that require correct time should use the
> Special Passive System Unit time-sync.target
>
> http://www.freedesktop.org/software/systemd/man/systemd.special.html#time-sync.target
> Signed-off-by: Jim Fehlig
> ---
>
> V2 of
>
> https://www.redhat.com/archives/libvir-list/2014-September/msg00426.html
>
> In V2: Use time-sync.target instead of ntp-wait.service
ACK
>
> tools/libvirt-guests.service.in | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in
> index d8d7adf..cc04b6d 100644
> --- a/tools/libvirt-guests.service.in
> +++ b/tools/libvirt-guests.service.in
> @@ -1,6 +1,6 @@
> [Unit]
> Description=Suspend Active Libvirt Guests
> -After=network.target libvirtd.service
> +After=network.target libvirtd.service time-sync.target
> Documentation=man:libvirtd(8)
> Documentation=http://libvirt.org
>
>
--
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
Now we have universal tunable event so we can use it for reporting
changes to user. The cputune values will be prefixed with "cputune" to
distinguish it from other tunable events.
Signed-off-by: Pavel Hrdina
---
since v4:
- added macro definitions for cputune typedParameters fileds
include/libvirt/libvirt.h.in | 63 ++
src/qemu/qemu_cgroup.c | 19 ++-
src/qemu/qemu_driver.c | 81
3 files changed, 162 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 86be86f..898f8b5 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5204,6 +5204,66 @@ typedef void
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
void *opaque);
/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN:
+ *
+ * Macro represents formatted pinning for one vcpu specified by id which is
+ * appended to the parameter name, for example "cputune.vcpupin1",
+ * as VIR_TYPED_PARAM_STRING.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN "cputune.vcpupin%u"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN:
+ *
+ * Macro represents formatted pinning for emulator process,
+ * as VIR_TYPED_PARAM_STRING.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN "cputune.emulatorpin"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES:
+ *
+ * Macro represents proportional weight of the scheduler used on the
+ * host cpu, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES "cputune.cpu_shares"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD:
+ *
+ * Macro represents the enforcement period for a quota, in microseconds,
+ * for vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD "cputune.vcpu_period"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA:
+ *
+ * Macro represents the maximum bandwidth to be used within a period for
+ * vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_LLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA "cputune.vcpu_quota"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD:
+ *
+ * Macro represents the enforcement period for a quota in microseconds,
+ * when using the posix scheduler, for all emulator activity not tied to
+ * vcpus, as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD "cputune.emulator_period"
+
+/**
+ * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA:
+ *
+ * Macro represents the maximum bandwidth to be used within a period for
+ * all emulator activity not tied to vcpus, when using the posix scheduler,
+ * as an VIR_TYPED_PARAM_LLONG.
+ */
+#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA "cputune.emulator_quota"
+
+
+/**
* virConnectDomainEventTunableCallback:
* @conn: connection object
* @dom: domain on which the event occurred
@@ -5215,6 +5275,9 @@ typedef void
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
* be freed in the callback handler as it's done internally after the callback
* handler is executed.
*
+ * Currently supported name spaces:
+ * "cputune.*"
+ *
* The callback signature to use when registering for an event of type
* VIR_DOMAIN_EVENT_ID_TUNABLE with virConnectDomainEventRegisterAny()
*/
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 7c6b2c1..41d7057 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -34,6 +34,7 @@
#include "virscsi.h"
#include "virstring.h"
#include "virfile.h"
+#include "virtypedparam.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -676,6 +677,10 @@ static int
qemuSetupCpuCgroup(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
+virObjectEventPtr event = NULL;
+virTypedParameterPtr eventParams = 0;
+int eventNparams = 0;
+int eventMaxparams = 0;
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
if (vm->def->cputune.sharesSpecified) {
@@ -694,7 +699,19 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
if (virCgroupGetCpuShares(priv->cgroup, &val) < 0)
return -1;
-vm->def->cputune.shares = val;
+if (vm->def->cputune.shares != val) {
+vm->def->cputune.shares = val;
+if (virTypedParamsAddULLong(&eventParams, &eventNparams,
+&eventMaxparams,
+VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES,
+val) < 0)
+return -1;
+
+event = virDomainEventTunableNewFromObj(vm, eventParams,
eventNparams);
+}
+
+if (event)
+qemuDomainEventQueue(vm->privateData, event);
}
return 0;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e73d4f9..d1a0657 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_dr
When libvirt-guests is configured to start guests on host
boot, it is possible for guests start and read the host
clock before it is synchronized. Services such as
libvirt-guests that require correct time should use the
Special Passive System Unit time-sync.target
http://www.freedesktop.org/software/systemd/man/systemd.special.html#time-sync.target
Signed-off-by: Jim Fehlig
---
V2 of
https://www.redhat.com/archives/libvir-list/2014-September/msg00426.html
In V2: Use time-sync.target instead of ntp-wait.service
tools/libvirt-guests.service.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in
index d8d7adf..cc04b6d 100644
--- a/tools/libvirt-guests.service.in
+++ b/tools/libvirt-guests.service.in
@@ -1,6 +1,6 @@
[Unit]
Description=Suspend Active Libvirt Guests
-After=network.target libvirtd.service
+After=network.target libvirtd.service time-sync.target
Documentation=man:libvirtd(8)
Documentation=http://libvirt.org
--
1.8.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Michal Privoznik wrote:
> On 23.09.2014 08:06, Martin Kletzander wrote:
>> On Mon, Sep 22, 2014 at 04:50:33PM -0600, Jim Fehlig wrote:
>>> Michal Privoznik wrote:
On 20.09.2014 01:36, Jim Fehlig wrote:
> Martin Kletzander wrote:
>> Unfortunately I'm not very familiar with systemd files, but my guess
>> is that After=ntp-wait.service means it should be started after the
>> time is synchronized if and only if the ntp-wait.service unit is
>> enabled, otherwise it doesn't require it.
>
> Yes, this is my understanding too.
And so is mine. The only concern I have is that syncing time on cold
boot of the host may take ages.
>>>
>>> Yep, I have this concern too. So I dug a bit further and see that
>>> ntp-wait (a perl script) scrapes the output of `ntpq -c "rv 0`, waiting
>>> for leap_alarm to change to leap_none, leap_add_sec, or leap_del_sec.
>>> On my test system, this took ~16min on cold boot :-(. ntp-wait.service
>>> failed in the meantime, since it by default calls /usr/sbin/ntp-wait
>>> with options to only wait 10min.
>>>
But on the other hand, it's better to start domains later and with
correct time than start asap with inaccurate time. ACK then,
>>>
>>> Given the above observations, I'll wait to see if you change your mind.
>>>
>>
>> What would you say to changing it to After=ntpdate.service? That way
>> it won't wait until the clock is synchronized, but it will be started
>> with proper time if ntpdate.service is set up to start in the default
>> runlevel (or is it target in systemd?). I think that's a compromise
>> that has no negative side-effects.
>
> I wonder if we should be this specific or use time-sync.target:
>
> time-sync.target
>
> Services responsible for synchronizing the system clock from a
> remote source (such as NTP client implementations) should pull in this
> target and order themselves before it. All services where correct time
> is essential should be ordered after this unit, but not pull it in.
> systemd automatically adds dependencies of type After= for this target
> unit to all SysV init script service units with an LSB header
> referring to the "$time" facility.
>
> http://www.freedesktop.org/software/systemd/man/systemd.special.html#time-sync.target
>
Cool, thanks for finding that!
>
> So I'd say: After=time-sync.target is what we are looking for.
Agreed. I'll send a V2.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Martin Kletzander wrote:
> On Mon, Sep 22, 2014 at 04:50:33PM -0600, Jim Fehlig wrote:
>> Michal Privoznik wrote:
>>> On 20.09.2014 01:36, Jim Fehlig wrote:
Martin Kletzander wrote:
> Unfortunately I'm not very familiar with systemd files, but my guess
> is that After=ntp-wait.service means it should be started after the
> time is synchronized if and only if the ntp-wait.service unit is
> enabled, otherwise it doesn't require it.
Yes, this is my understanding too.
>>>
>>> And so is mine. The only concern I have is that syncing time on cold
>>> boot of the host may take ages.
>>
>> Yep, I have this concern too. So I dug a bit further and see that
>> ntp-wait (a perl script) scrapes the output of `ntpq -c "rv 0`, waiting
>> for leap_alarm to change to leap_none, leap_add_sec, or leap_del_sec.
>> On my test system, this took ~16min on cold boot :-(. ntp-wait.service
>> failed in the meantime, since it by default calls /usr/sbin/ntp-wait
>> with options to only wait 10min.
>>
>>> But on the other hand, it's better to start domains later and with
>>> correct time than start asap with inaccurate time. ACK then,
>>
>> Given the above observations, I'll wait to see if you change your mind.
>>
>
> What would you say to changing it to After=ntpdate.service?
It appears ntpdate is deprecated
https://support.ntp.org/bin/view/Dev/DeprecatingNtpdate
Like ifconfig, it is having a very slow death.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Right now when building the qemu command line, we try to do various
unconditional validations of the guest CPU against the host CPU. However
this checks are overly applied. The only time we should use the checks
are:
- The user requests host-model/host-passthrough, or
- When KVM is requsted. CPU features requested in TCG mode are always
emulated by qemu and are independent of the host CPU, so no host CPU
checks should be performed.
Right now if trying to specify a CPU for arm on an x86 host, it attempts
to do non-sensical validation and falls over.
Switch all the test cases that were intending to test CPU validation to
use KVM, so they continue to test the intended code.
Amend some aarch64 XML tests with a CPU model, to ensure things work
correctly.
---
src/qemu/qemu_command.c| 68 +-
.../qemuxml2argv-aarch64-virt-default-nic.args | 3 +-
.../qemuxml2argv-aarch64-virt-default-nic.xml | 3 +
.../qemuxml2argv-aarch64-virt-virtio.args | 3 +-
.../qemuxml2argv-aarch64-virt-virtio.xml | 3 +
.../qemuxml2argvdata/qemuxml2argv-cpu-exact1.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml | 4 +-
.../qemuxml2argv-cpu-exact2-nofallback.args| 2 +-
.../qemuxml2argv-cpu-exact2-nofallback.xml | 4 +-
.../qemuxml2argvdata/qemuxml2argv-cpu-exact2.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml | 4 +-
.../qemuxml2argv-cpu-fallback.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml | 4 +-
.../qemuxml2argv-cpu-minimum1.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml | 4 +-
.../qemuxml2argv-cpu-minimum2.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml | 4 +-
.../qemuxml2argv-cpu-nofallback.xml| 2 +-
.../qemuxml2argvdata/qemuxml2argv-cpu-strict1.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml | 4 +-
.../qemuxml2argv-graphics-spice-timeout.args | 2 +-
.../qemuxml2argv-graphics-spice-timeout.xml| 4 +-
.../qemuxml2argv-pseries-cpu-exact.args| 4 +-
tests/qemuxml2argvtest.c | 21 +++
.../qemuxml2xmlout-graphics-spice-timeout.xml | 4 +-
25 files changed, 90 insertions(+), 69 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7c43604..a84676a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6116,6 +6116,8 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
virCPUCompareResult cmp;
const char *preferred;
virCapsPtr caps = NULL;
+bool compareAgainstHost = (def->virtType == VIR_DOMAIN_VIRT_KVM ||
+def->cpu->mode != VIR_CPU_MODE_CUSTOM);
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
@@ -6138,30 +6140,33 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
cpuUpdate(cpu, host) < 0)
goto cleanup;
-cmp = cpuGuestData(host, cpu, &data, &compare_msg);
-switch (cmp) {
-case VIR_CPU_COMPARE_INCOMPATIBLE:
-if (compare_msg) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("guest and host CPU are not compatible: %s"),
- compare_msg);
-} else {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("guest CPU is not compatible with host CPU"));
-}
-/* fall through */
-case VIR_CPU_COMPARE_ERROR:
-goto cleanup;
+/* For non-KVM, CPU features are emulated, so host compat doesn't matter */
+if (compareAgainstHost) {
+cmp = cpuGuestData(host, cpu, &data, &compare_msg);
+switch (cmp) {
+case VIR_CPU_COMPARE_INCOMPATIBLE:
+if (compare_msg) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("guest and host CPU are not compatible: %s"),
+ compare_msg);
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("guest CPU is not compatible with host CPU"));
+}
+/* fall through */
+case VIR_CPU_COMPARE_ERROR:
+goto cleanup;
-default:
-break;
+default:
+break;
+}
}
/* Only 'svm' requires --enable-nesting. The nested
* 'vmx' patches now simply hook off the CPU features
*/
-if (def->os.arch == VIR_ARCH_X86_64 ||
-def->os.arch == VIR_ARCH_I686) {
+if ((def->os.arch == VIR_ARCH_X86_64 || def->os.arch == VIR_ARCH_I686) &&
+ compareAgainstHost) {
int hasSVM = cpuHasFeature(data, "svm");
if (hasSVM < 0)
goto cleanup;
@@ -6189,16 +6194,23 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
if (VIR_STRDUP(guest->vendor_id, cpu->vendor_id) < 0)
goto cleanup;
-
Qemu currently implements pre-copy live migration. VM memory pages are
first copied from the source hypervisor to the destination, potentially
multiple times as pages get dirtied during transfer, then VCPU state
is migrated. Unfortunately, if the VM dirties memory faster than the
network bandwidth, then pre-copy cannot finish. `virsh` currently
includes an option to suspend a VM after a timeout, so that migration
may finish, but at the expense of downtime.
A future version of qemu will implement post-copy live migration. The
VCPU state is first migrated to the destination hypervisor, then
memory pages are pulled from the source hypervisor. Post-copy has the
potential to do migration with zero-downtime, despite the VM dirtying
pages fast, with minimum performance impact. On the other hand, one
post-copy is in progress, any network failure would render the VM
unusable, as its memory is partitioned between the source and
destination hypervisor. Therefore, post-copy should only be used when
necessary.
Post-copy migration in qemu will work as follows:
(1) The `x-postcopy-ram` migration capability needs to be set.
(2) Migration is started.
(3) When the user decides so, post-copy migration is activated by
sending the `migrate-start-postcopy` command. Qemu acknowledges by
setting migration status to `postcopy-active`.
Cristian Klein (6):
Added qemu postcopy-active migration status
Added public API to enable post-copy migration
Added new public API virDomainMigrateStartPostCopy
Added domainMigrateStartPostCopy in qemu driver
Added domainMigrateStartPostCopy in remote driver
virsh: added migrate --postcopy-after
include/libvirt/libvirt.h.in | 3 ++
src/driver.h | 4 +++
src/libvirt.c| 44 +++
src/libvirt_public.syms | 5 +++
src/qemu/qemu_driver.c | 44 +++
src/qemu/qemu_migration.c| 44 +++
src/qemu/qemu_migration.h| 3 +-
src/qemu/qemu_monitor.c | 21 -
src/qemu/qemu_monitor.h | 3 ++
src/qemu/qemu_monitor_json.c | 21 -
src/qemu/qemu_monitor_json.h | 1 +
src/qemu/qemu_monitor_text.c | 14 -
src/qemu/qemu_monitor_text.h | 2 ++
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 12 +++-
tools/virsh-domain.c | 72 ++--
tools/virsh.pod | 5 +++
17 files changed, 292 insertions(+), 7 deletions(-)
--
1.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/18/2014 10:24 AM, Michal Privoznik wrote:
> A long time ago in a galaxy far, far away it has been decided
> that libvirt will manage not only domains but host as well. And
> with my latest work on qemu driver supporting huge pages, we miss
> the cherry on top: an API to allocate huge pages on the run.
> Currently users are forced to log into the host and adjust the
> huge pages pool themselves. However, with this API the problem
> is gone - they can both size up and size down the pool.
>
> Signed-off-by: Michal Privoznik
> ---
> daemon/remote.c | 37
> include/libvirt/libvirt.h.in | 16 +++
> src/driver.h | 10 +++
> src/libvirt.c| 67
>
> src/libvirt_public.syms | 1 +
> src/remote/remote_driver.c | 47 +++
> src/remote/remote_protocol.x | 20 -
> src/remote_protocol-structs | 17 +++
> 8 files changed, 214 insertions(+), 1 deletion(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 724314e..1a47bae 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -5505,6 +5505,22 @@ int virNodeGetFreePages(virConnectPtr conn,
> unsigned int cellcount,
> unsigned long long *counts,
> unsigned int flags);
> +
> +typedef enum {
> +VIR_NODE_ALLOC_PAGES_ADD = 0, /* Add @pageCounts to the pages pool. This
> + can be used only to size up the pool. */
> +VIR_NODE_ALLOC_PAGES_SET = (1 << 0), /* Don't add @pageCounts, instead
> set
> +passed number of pages. This can
> be
> +used to free allocated pages. */
> +} virNodeAllocPagesFlags;
> +
> +int virNodeAllocPages(virConnectPtr conn,
> + unsigned int npages,
> + unsigned int *pageSizes,
> + unsigned long long *pageCounts,
> + int startCell,
> + unsigned int cellCount,
> + unsigned int flags);
> /**
> * virSchedParameterType:
> *
> diff --git a/src/driver.h b/src/driver.h
> index bb748c4..dc62a8e 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -1212,6 +1212,15 @@ typedef int
>virDomainStatsRecordPtr **retStats,
>unsigned int flags);
>
> +typedef int
> +(*virDrvNodeAllocPages)(virConnectPtr conn,
> +unsigned int npages,
> +unsigned int *pageSizes,
> +unsigned long long *pageCounts,
> +int startCell,
> +unsigned int cellCount,
> +unsigned int flags);
> +
> typedef struct _virDriver virDriver;
> typedef virDriver *virDriverPtr;
>
> @@ -1435,6 +1444,7 @@ struct _virDriver {
> virDrvNodeGetFreePages nodeGetFreePages;
> virDrvConnectGetDomainCapabilities connectGetDomainCapabilities;
> virDrvConnectGetAllDomainStats connectGetAllDomainStats;
> +virDrvNodeAllocPages nodeAllocPages;
> };
>
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 7c63825..27445e5 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -21841,3 +21841,70 @@ virDomainStatsRecordListFree(virDomainStatsRecordPtr
> *stats)
>
> VIR_FREE(stats);
> }
> +
> +
> +/**
> + * virNodeAllocPages:
> + * @conn: pointer to the hypervisor connection
> + * @npages: number of items in the @pageSizes array
> + * @pageSizes: which huge page sizes to allocate
> + * @pageCounts: how many pages should be allocated
> + * @startCell: index of first cell to allocate pages on
> + * @cellCount: number of consecutive cells to allocate pages on
> + * @flags: extra flags; binary-OR of virConnectGetAllDomainStatsFlags
virNodeAllocPagesFlags
> + *
> + * Sometimes, when trying to start a new domain, it may be
> + * necessary to reserve some huge pages in the system pool which
> + * can be then allocated by the domain. This API serves that
> + * purpose. On its input, @pageSizes and @pageCounts are arrays
> + * of the same cardinality of @npages. The @pageSizes contains
> + * page sizes which are to be allocated in the system (the size
> + * unit is kibibytes), and @pageCounts then contains the number
> + * of pages to reserve. Depending on @flags specified, the number
> + * of pages is either added into the pool and the pool is sized
> + * up (@flags have VIR_NODE_ALLOC_PAGES_ADD set) or the number of
> + * pages is looked at the new size of pool and the pool can be
> + * both sized up or down (@flags have VIR_NODE_ALLOC_PAGES_SET
> + * set). The pages pool can be allocated over several NUMA nodes
> + * at once, just point at @startCell and tell how many subsequent
> + * NUMA nodes shou
On 23.09.2014 05:40, Chen Hanxiao wrote:
We don't accept any other values except '0'.
Signed-off-by: Chen Hanxiao
---
docs/formatdomain.html.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index eefdd5e..bb72452 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -315,7 +315,7 @@
start
- First user ID in container.
+ First user ID in container. It must be '0'.
target
The first user ID in container will be mapped to this target user
ID in host.
This is not entirely true. Only the first id mapping must refer to root.
And by first I mean in a sorted array of mappings. For instance:
is okay.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 23.09.2014 11:34, Chen Fan wrote:
For memnode in numatune element, the range of attribute 'nodeset'
was not validated. on my host maxnodes was 1, but when setting nodeset
to '0-2' or more, guest also started succuss. there probably was qemu's
bug too.
Signed-off-by: Chen Fan
---
src/conf/numatune_conf.c | 29 +
src/conf/numatune_conf.h | 4
2 files changed, 33 insertions(+)
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 21d9a64..a9b20aa 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -183,6 +183,9 @@ virDomainNumatuneNodeParseXML(virDomainNumatunePtr
*numatunePtr,
VIR_DOMAIN_CPUMASK_LEN) < 0)
goto cleanup;
VIR_FREE(tmp);
+
+if (!virDomainNumatuneNodeSetIsAvailable(numatune, cellid))
+goto cleanup;
Well, if there already exists such configuration within an existing
domain, this will cause a failure on XML parsing when the daemon is
starting and hence domain is lost.
}
ret = 0;
@@ -612,3 +615,29 @@ virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr
numatune)
return false;
}
+
+bool
+virDomainNumatuneNodeSetIsAvailable(virDomainNumatunePtr numatune,
+int cellid)
+{
+int maxnode;
+int bit = -1;
+virBitmapPtr nodemask = NULL;
+
+nodemask = virDomainNumatuneGetNodeset(numatune, NULL, cellid);
+if (!nodemask)
+return false;
+
+if ((maxnode = virNumaGetMaxNode()) < 0)
+return false;
This will work in real environment, but won't work in tests. You need to
mock this to get predictable max numa node.
+
+while ((bit = virBitmapNextSetBit(nodemask, bit)) >= 0) {
+if (bit > maxnode) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("NUMA node %d is out of range"), bit);
+return false;
+}
+}
+
+return true;
+}
diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
index 5254629..cab0b83 100644
--- a/src/conf/numatune_conf.h
+++ b/src/conf/numatune_conf.h
@@ -102,4 +102,8 @@ bool virDomainNumatuneHasPlacementAuto(virDomainNumatunePtr
numatune);
bool virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr numatune);
+extern int virNumaGetMaxNode(void);
+bool virDomainNumatuneNodeSetIsAvailable(virDomainNumatunePtr numatune,
+ int cellid);
+
#endif /* __NUMATUNE_CONF_H__ */
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Add defines for the news options total_bytes_sec_max, write_bytes_sec_max,
read_bytes_sec_max
total_iops_sec_max, write_iops_sec_max, read_iops_sec_max, size_iops_sec.
Modify the structure _virDomainBlockIoTuneInfo to support these options.
Change the initialization of the variable expectedInfo in qemumonitorjsontest.c
to avoid compiling problem.
Allow libvirt to save the configuration.
Signed-off-by: Matthias Gatto
---
include/libvirt/libvirt.h.in | 54 +++
src/conf/domain_conf.c | 89 +++-
src/conf/domain_conf.h | 7
tests/qemumonitorjsontest.c | 2 +-
4 files changed, 150 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index ad6785f..4094103 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2668,6 +2668,60 @@ int virDomainBlockCommit(virDomainPtr dom, const char
*disk, const char *base,
*/
#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC "write_iops_sec"
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the maximum total
+ * bytes per second permitted through a block device, as a ullong.
+ */
+#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX "total_bytes_sec_max"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the maximum read
+ * bytes per second permitted through a block device, as a ullong.
+ */
+#define VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX "read_bytes_sec_max"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the maximum write
+ * bytes per second permitted through a block device, as a ullong.
+ */
+#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX "write_bytes_sec_max"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the maximum
+ * I/O operations per second permitted through a block device, as a ullong.
+ */
+#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX "total_iops_sec_max"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the maximum read
+ * I/O operations per second permitted through a block device, as a ullong.
+ */
+#define VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX "read_iops_sec_max"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX:
+ * Macro for the BlockIoTune tunable weight: it represents the maximum write
+ * I/O operations per second permitted through a block device, as a ullong.
+ */
+#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX "write_iops_sec_max"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC:
+ * Macro for the BlockIoTune tunable weight: it represents the size
+ * I/O operations per second permitted through a block device, as a ullong.
+ */
+#define VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC "size_iops_sec"
+
int
virDomainSetBlockIoTune(virDomainPtr dom,
const char *disk,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c7016f3..57b9e7f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5680,6 +5680,49 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
def->blkdeviotune.write_iops_sec = 0;
}
+if (virXPathULongLong("string(./iotune/total_bytes_sec_max)",
+ ctxt,
+ &def->blkdeviotune.total_bytes_sec_max)
< 0) {
+def->blkdeviotune.total_bytes_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/read_bytes_sec_max)",
+ ctxt,
+ &def->blkdeviotune.read_bytes_sec_max) <
0) {
+def->blkdeviotune.read_bytes_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/write_bytes_sec_max)",
+ ctxt,
+ &def->blkdeviotune.write_bytes_sec_max)
< 0) {
+def->blkdeviotune.write_bytes_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/total_iops_sec_max)",
+ ctxt,
+ &def->blkdeviotune.total_iops_sec_max) <
0) {
+def->blkdeviotune.total_iops_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/read_iops_sec_max)",
+ ctxt,
+ &def->blkdeviotune.read_iops_sec_max) <
0) {
+def->blkdeviotune.read_iops_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/write_iops_sec_max)",
+
This series of patches add support for bps_max, bps_rd_max, bps_wr_max,
bps_max, bps_rd_max, bps_wr_max, and iops_size in the functions
qemuDomainSetBlockIoTune and qemuDomainGetBlockIoTune.
The last patch add support for these parameters to the virsh blkdeviotune
command.
v2: spellfix
v3: Merge patch 1/9,2/9,5/9 together.
Change the capability detection.(patch 2/7 and 3/7).
Try to make the usage of QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX more
explicit(patch 3/7).
Matthias Gatto (7):
qemu: Add defines for the news throttle options and modify the
structure _virDomainBlockIoTuneInfo.
qemu: Add the capability to detect if the qemu binary have the
capability to use bps_max and friends
qemu: Add bps_max and friends qemu driver
qemu: Add bps_max and friends QMP suport
qemu: Add bps_max and friends "text" support
qemu: add bps_max and friends to qemu command generation
virsh: Add bps_max and friends to virsh
include/libvirt/libvirt.h.in | 54 +++
src/conf/domain_conf.c | 89 +++-
src/conf/domain_conf.h | 7 ++
src/qemu/qemu_capabilities.c | 4 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 49 +
src/qemu/qemu_driver.c | 162 ---
src/qemu/qemu_monitor.c | 14 ++--
src/qemu/qemu_monitor.h | 6 +-
src/qemu/qemu_monitor_json.c | 70 +++
src/qemu/qemu_monitor_json.h | 6 +-
src/qemu/qemu_monitor_text.c | 89 +---
src/qemu/qemu_monitor_text.h | 6 +-
tests/qemumonitorjsontest.c | 6 +-
tools/virsh-domain.c | 119 +++
tools/virsh.pod | 11 ++-
16 files changed, 643 insertions(+), 50 deletions(-)
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 19.09.2014 18:14, Serge Hallyn wrote:
The security_driver line in /etc/libvirt/qemu.conf is best-effort - if
selinux is not available on the host, then 'none' will be used.
The security_driver line in /etc/libvirt/lxc.conf doesn't behave the
same way - if apparmor is specified but policies are not available
on the host, then container creation fails.
This patch always tries to fall back to 'none' if the requested
driver is not available. A better patch would allow an option list
like qemu.conf allows, but this patch doesn't do that.
Signed-off-by: Serge Hallyn
---
src/lxc/lxc_driver.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c3cd62c..233e558 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1541,6 +1541,11 @@ lxcSecurityInit(virLXCDriverConfigPtr cfg)
cfg->securityDefaultConfined,
cfg->securityRequireConfined);
if (!mgr)
+mgr = virSecurityManagerNew(NULL, LXC_DRIVER_NAME, false,
+
cfg->securityDefaultConfined,
+
cfg->securityRequireConfined);
+
+if (!mgr)
goto error;
return mgr;
IIUC the code, the new sec manager is created from
cfg->securityDriverName which has no default value. It contains only
what user specified in lxc.conf. Well, if user sets apparmor there but
it's not available for whatever reason, we must fail and not workaround it.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/23/2014 07:12 AM, Michal Privoznik wrote:
> In the function at one place we check if def->cpu is NULL prior
> to accessing def->cpu->ncells. Then, later in the code,
> def->cpu->ncells is accessed directly, without the check. This
> makes coverity unhappy, because the first check makes it think
> def->cpu can be NULL. However, the function is not called if
> def->cpu is NULL. Therefore, remove the first check and hopefully
> make coverity cheer again.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_command.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
It did... ACK
Tks -
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/23/2014 05:13 AM, Pavel Hrdina wrote:
> On 09/22/2014 05:50 PM, John Ferlan wrote:
>>
>>
>> On 09/18/2014 04:39 AM, Pavel Hrdina wrote:
>>> This new event will use typedParameters to expose what has been actually
>>> updated and the reason is that we can in the future extend any tunable
>>> values or add new tunable values. With typedParameters we don't have to
>>> worry about creating some other events, we will just use this universal
>>> event to inform user about updates.
>>>
>>> Signed-off-by: Pavel Hrdina
>>> ---
>>> daemon/remote.c | 45 +
>>> include/libvirt/libvirt.h.in | 22 +++
>>> src/conf/domain_event.c | 93
>>>
>>> src/conf/domain_event.h | 9 +
>>> src/libvirt_private.syms | 2 +
>>> src/remote/remote_driver.c | 42
>>> src/remote/remote_protocol.x | 14 ++-
>>> src/remote_protocol-structs | 9 +
>>> tools/virsh-domain.c | 33
>>> 9 files changed, 268 insertions(+), 1 deletion(-)
>>>
>>
>> Seems you covered previous code review comments just fine, although I
>> have a couple of questions
>>
>> #1. Is virsh-domain.c relative here or perhaps later? Or should the
>> verb/action be changed from "cpu-tune" to something more generic?
>> "tunable"?
>>
>
> That should be definitely changed to "tunable". Thanks for catching it.
>
>> #2. Should the REMOTE_CPUMAPS_MAX be replaced by something more name
>> (and size) relative to these events? And secondarily, since event max
>> params is relevant to the size of the params buffer allocated for each
>> event series shouldn't that be passed along and eventually used as the
>> limit for the remoteDeserializeTypedParameters? Although I'm not quite
>> clear/sure how that works with the protocol definition file.
>>
>
> And this one is also wrong and should definitely be
> REMOTE_DOMAIN_EVENT_TUNABLE_MAX and that means create a new const. About
> the size of the rpc message I should extend the size as in the future
> there could be some tunable staff that will require more memory than the
> cputune event.
>
> Currently maximal size of any rpc message is 16MiB and I think that 8MiB
> should by more than enough limit for the tunable event, what do you think?
The "right size" is always a bit tough to judge on these generic events.
I'm fine with the 8MiB value - just seems large though. I see why
cpumaps max is large since it's a text mapping of the 'arbitrary' number
of cpus (on different archs)... Given the recent add of the all domain
stats, my mind went to how much more data than that could be necessary,
but in this case I guess substance also matters.
John
>
> Pavel
>
>> I think this is ACK-able - just want to make sure of the above - #1
>> should be 'simple' right? #2 is just one of those size things - the
>> current usage goes on a byte max, but is that relative to how this has
>> changed/evolved.
>>
>> John
>>
>
> [...]
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
In the function at one place we check if def->cpu is NULL prior
to accessing def->cpu->ncells. Then, later in the code,
def->cpu->ncells is accessed directly, without the check. This
makes coverity unhappy, because the first check makes it think
def->cpu can be NULL. However, the function is not called if
def->cpu is NULL. Therefore, remove the first check and hopefully
make coverity cheer again.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d470b1b..65864d2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6594,7 +6594,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
continue;
}
-if (def->cpu && def->cpu->ncells) {
+if (def->cpu->ncells) {
/* Fortunately, we allow only guest NUMA nodes to be continuous
* starting from zero. */
pos = def->cpu->ncells - 1;
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
for memnode.nodeset in numatune, when setting it more
than the host nodes, it should fail.
Chen Fan (2):
numatune: add check for memnode.nodeset range
numatune: move up verification codes in virNumaSetupMemoryPolicy
src/conf/numatune_conf.c | 32
src/conf/numatune_conf.h | 4
src/util/virnuma.c | 15 ---
3 files changed, 36 insertions(+), 15 deletions(-)
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/22/2014 05:50 PM, John Ferlan wrote:
On 09/18/2014 04:39 AM, Pavel Hrdina wrote:
This new event will use typedParameters to expose what has been actually
updated and the reason is that we can in the future extend any tunable
values or add new tunable values. With typedParameters we don't have to
worry about creating some other events, we will just use this universal
event to inform user about updates.
Signed-off-by: Pavel Hrdina
---
daemon/remote.c | 45 +
include/libvirt/libvirt.h.in | 22 +++
src/conf/domain_event.c | 93
src/conf/domain_event.h | 9 +
src/libvirt_private.syms | 2 +
src/remote/remote_driver.c | 42
src/remote/remote_protocol.x | 14 ++-
src/remote_protocol-structs | 9 +
tools/virsh-domain.c | 33
9 files changed, 268 insertions(+), 1 deletion(-)
Seems you covered previous code review comments just fine, although I
have a couple of questions
#1. Is virsh-domain.c relative here or perhaps later? Or should the
verb/action be changed from "cpu-tune" to something more generic?
"tunable"?
That should be definitely changed to "tunable". Thanks for catching it.
#2. Should the REMOTE_CPUMAPS_MAX be replaced by something more name
(and size) relative to these events? And secondarily, since event max
params is relevant to the size of the params buffer allocated for each
event series shouldn't that be passed along and eventually used as the
limit for the remoteDeserializeTypedParameters? Although I'm not quite
clear/sure how that works with the protocol definition file.
And this one is also wrong and should definitely be
REMOTE_DOMAIN_EVENT_TUNABLE_MAX and that means create a new const. About
the size of the rpc message I should extend the size as in the future
there could be some tunable staff that will require more memory than the
cputune event.
Currently maximal size of any rpc message is 16MiB and I think that 8MiB
should by more than enough limit for the tunable event, what do you think?
Pavel
I think this is ACK-able - just want to make sure of the above - #1
should be 'simple' right? #2 is just one of those size things - the
current usage goes on a byte max, but is that relative to how this has
changed/evolved.
John
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 22.09.2014 16:02, Gerald Palmer wrote:
Description of problem: Fail to compile
Version-Release number of selected component (if applicable):
Version 1.0.3 through 1.1.1
How reproducible:
Steps to Reproduce:
1./configure --prefix=/usr --localstatedir=/var --sysconfdir=/etc
2.make
Actual results:
libvirt_lxc-lxc_monitor_protocol.o: In function
`xdr_virLXCMonitorInitEventMsg':
/usr/local/src/libvirt/libvirt-1.1.1/src/./lxc/lxc_monitor_protocol.c:31:
undefined
reference to `xdr_uint64_t'
Expected results:
Successful Compilation
Additional info: patch resolving compilation issue
--- lxc_monitor_protocol.h 2013-08-05 20:22:23.96300 +
+++ libvirt-1.1.1/src/lxc/lxc_monitor_protocol.h 2013-08-05
20:22:46.22600 +
@@ -5,6 +5,8 @@
#ifndef _LXC_MONITOR_PROTOCOL_H_RPCGEN
#define _LXC_MONITOR_PROTOCOL_H_RPCGEN
+# define xdr_uint64_t xdr_u_int64_t
+
A few lines above this part is a notice, that this file is generated and
you should not edit this file. I'm proposing a patch that fixes the file
from which lxc_monitor_protocol.h is generated.
https://www.redhat.com/archives/libvir-list/2014-September/msg01360.html
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/22/2014 05:47 PM, John Ferlan wrote:
On 09/18/2014 04:39 AM, Pavel Hrdina wrote:
Cleanup virDomanDef structure from other nested structure and create
separate type definition for them.
Fix a typo in virDomainHugePage.
Signed-off-by: Pavel Hrdina
---
src/conf/domain_conf.h | 102 +
1 file changed, 61 insertions(+), 41 deletions(-)
ACK
John
Thanks, pushed.
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/19/2014 06:47 PM, Kevin Wolf wrote:> Am 16.09.2014 um 14:59 hat Paolo
Bonzini geschrieben:
>> Il 16/09/2014 14:52, Kevin Wolf ha scritto:
>>> Yes, that's true. We can't fix this problem in qcow2, though, because
>>> it's a more general one. I think we must make sure that
>>> bdrv_invalidate_cache() doesn't yield.
>>>
>>> Either by forbidding to run bdrv_invalidate_cache() in a coroutine and
>>> moving the problem to the caller (where and why is it even called from a
>>> coroutine?), or possibly by creating a new coroutine for the driver
>>> callback and running that in a nested event loop that only handles
>>> bdrv_invalidate_cache() callbacks, so that the NBD server doesn't get a
>>> chance to process new requests in this thread.
>>
>> Incoming migration runs in a coroutine (the coroutine entry point is
>> process_incoming_migration_co). But everything after qemu_fclose() can
>> probably be moved into a separate bottom half, so that it gets out of
>> coroutine context.
>
> Alexey, you should probably rather try this (and add a bdrv_drain_all()
> in bdrv_invalidate_cache) than messing around with qcow2 locks. This
> isn't a problem that can be completely fixed in qcow2.
Ok. Tried :) Not very successful though. The patch is below.
Is that the correct bottom half? When I did it, I started getting crashes
in various sport on accesses to s->l1_cache which is NULL after qcow2_close.
Normally the code would check s->l1_size and then use but they are out of sync.
So I clear it in qcow2_close(). This allowed migrated guest to work and not
to crash until I shut it down when it aborted at "HERE IT FAILS ON SHUTDOWN".
Here I realized I am missing something in this picture again, what is it?
Thanks!
---
block.c | 2 ++
block/qcow2-cache.c | 2 +-
block/qcow2.c | 50 --
block/qcow2.h | 4
4 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/block.c b/block.c
index d06dd51..1e6dfd1 100644
--- a/block.c
+++ b/block.c
@@ -5044,6 +5044,8 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error
**errp)
error_setg_errno(errp, -ret, "Could not refresh total sector count");
return;
}
+
+bdrv_drain_all();
}
void bdrv_invalidate_cache_all(Error **errp)
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 904f6b1..59ff48c 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -259,7 +259,7 @@ static int qcow2_cache_find_entry_to_replace(Qcow2Cache *c)
if (min_index == -1) {
/* This can't happen in current synchronous code, but leave the check
* here as a reminder for whoever starts using AIO with the cache */
-abort();
+abort(); // < HERE IT FAILS ON SHUTDOWN
}
return min_index;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index f9e045f..2b84562 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1399,6 +1399,7 @@ static void qcow2_close(BlockDriverState *bs)
qemu_vfree(s->l1_table);
/* else pre-write overlap checks in cache_destroy may crash */
s->l1_table = NULL;
+s->l1_size = 0;
if (!(bs->open_flags & BDRV_O_INCOMING)) {
qcow2_cache_flush(bs, s->l2_table_cache);
@@ -1419,16 +1420,11 @@ static void qcow2_close(BlockDriverState *bs)
qcow2_free_snapshots(bs);
}
+static void qcow2_invalidate_cache_bh_cb(void *opaque);
+
static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
{
BDRVQcowState *s = bs->opaque;
-int flags = s->flags;
-AES_KEY aes_encrypt_key;
-AES_KEY aes_decrypt_key;
-uint32_t crypt_method = 0;
-QDict *options;
-Error *local_err = NULL;
-int ret;
/*
* Backing files are read-only which makes all of their metadata immutable,
@@ -1436,13 +1432,28 @@ static void qcow2_invalidate_cache(BlockDriverState
*bs, Error **errp)
*/
if (s->crypt_method) {
-crypt_method = s->crypt_method;
-memcpy(&aes_encrypt_key, &s->aes_encrypt_key, sizeof(aes_encrypt_key));
-memcpy(&aes_decrypt_key, &s->aes_decrypt_key, sizeof(aes_decrypt_key));
+s->bh_crypt_method = s->crypt_method;
+memcpy(&s->bh_aes_encrypt_key, &s->aes_encrypt_key,
sizeof(s->bh_aes_encrypt_key));
+memcpy(&s->bh_aes_decrypt_key, &s->aes_decrypt_key,
sizeof(s->bh_aes_decrypt_key));
+} else {
+s->bh_crypt_method = 0;
}
qcow2_close(bs);
+s->cache_inv_bh = aio_bh_new(bdrv_get_aio_context(bs),
+ qcow2_invalidate_cache_bh_cb,
+ bs);
+}
+
+static void qcow2_invalidate_cache_bh(BlockDriverState *bs, Error **errp)
+{
+BDRVQcowState *s = bs->opaque;
+int flags = s->flags;
+QDict *options;
+Error *local_err = NULL;
+int ret;
+
bdrv_invalidate_cache(bs->file, &local_err);
if (local_err) {
error_propagate(errp, local_err);
@@ -1464,11 +1475,22 @@ static void qcow2_invalidate_cache(Bl
On 22.09.2014 12:14, Jincheng Miao wrote:
Jincheng Miao (3):
virsh-host: fix pagesize unit of freepages
nodeinfo: report error when given node is out of range
Fix typo of virNodeGetFreePages comment
src/libvirt.c | 2 +-
src/nodeinfo.c | 24 +---
tools/virsh-host.c | 27 ++-
3 files changed, 40 insertions(+), 13 deletions(-)
Basically ACK to all three patches. I'm changing 1/3 and 2/3 according
to my comments and pushing. Good job.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 22.09.2014 12:14, Jincheng Miao wrote:
Signed-off-by: Jincheng Miao
---
src/libvirt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 7c63825..aa83365 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21307,7 +21307,7 @@ virDomainSetTime(virDomainPtr dom,
* @flags: extra flags; not used yet, so callers should always pass 0
*
* This calls queries the host system on free pages of
- * specified size. Ont the input, @pages is expected to be
+ * specified size. For the input, @pages is expected to be
* filled with pages that caller is interested in (the size
* unit is kibibytes, so e.g. pass 2048 for 2MB), then @startcell
* refers to the first NUMA node that info should be collected
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Sep 23, 2014 at 09:59:17AM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" writes:
>
> > On Mon, Sep 22, 2014 at 05:32:16PM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" writes:
> >>
> >> > On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
> >> >> Add a configure option --enable-pc-1-0-qemu-kvm and the
> >> >> corresponding --disable-pc-1-0-qemu-kvm, defaulting
> >> >> to disabled.
> >> >>
> >> >> Rename machine type pc-1.0 to pc-1.0-qemu-git.
> >> >>
> >> >> Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm
> >> >> or pc-1.0-qemu-git depending on the value of the config
> >> >> option.
> >> >>
> >> >> Signed-off-by: Alex Bligh
> >> >
> >> > I have to say, this one bothers me.
> >> > We end up not being able to predict what does pc-1.0
> >> > reference.
> >> >
> >> > Users also don't get qemu from git so I don't see
> >> > why does git make sense in the name?
> >> >
> >> > Legacy management applications invoked qemu as qemu-kvm -
> >> > how about detecting that name and switching
> >> > the machine types?
> >>
> >> Ugh! I like that even less than a configure option.
> >
> > configure options are tested even less than runtime ones:
> > each distro has to pick up a set of options and all
> > users are stuck with it.
> > So let's assume Ubuntu sets this and something is broken. How is a
> > Fedora user to test this? Find out configure flags that Ubuntu uses
> > and rebuild from source? It's hard to get people to triage bugs as it
> > is.
> > That's why changing behaviour in subtle ways depending on
> > configure options is a very bad idea.
>
> All it changes is a default machine type. I wouldn't classify that as
> "very bad". Fortunately, our difference of opinion doesn't matter,
> because the conclusion of the thread is "leave it to the distros".
>
> > So a flag might be better, but if we want to be compatible
> > with qemu-kvm, poking at argv[0] still better than configure.
>
> That I do consider a genuinely bad idea.
>
> Example of usage messed up by it: I symlink from my ~/bin to executables
> in various build trees. If one of these programs changed behavior
> depending on what it finds in argv[0], I'd have to know *exactly* how it
> matches argv[0] to choose suitable names for my symlinks.
>
> The name "qemu-kvm" is not a useful indicator of compatibility with the
> qemu-kvm fork of QEMU anyway. There are programs out there calling
> themselves qemu-kvm that aren't compatible with the qemu-kvm fork. And
> there are programs out there that are compatible, but call themselves
> something else.
I think the approach v4 takes is reasonable, though.
I didn't look at the implementation yet.
--
MST
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
"Michael S. Tsirkin" writes:
> On Mon, Sep 22, 2014 at 05:32:16PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" writes:
>>
>> > On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
>> >> Add a configure option --enable-pc-1-0-qemu-kvm and the
>> >> corresponding --disable-pc-1-0-qemu-kvm, defaulting
>> >> to disabled.
>> >>
>> >> Rename machine type pc-1.0 to pc-1.0-qemu-git.
>> >>
>> >> Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm
>> >> or pc-1.0-qemu-git depending on the value of the config
>> >> option.
>> >>
>> >> Signed-off-by: Alex Bligh
>> >
>> > I have to say, this one bothers me.
>> > We end up not being able to predict what does pc-1.0
>> > reference.
>> >
>> > Users also don't get qemu from git so I don't see
>> > why does git make sense in the name?
>> >
>> > Legacy management applications invoked qemu as qemu-kvm -
>> > how about detecting that name and switching
>> > the machine types?
>>
>> Ugh! I like that even less than a configure option.
>
> configure options are tested even less than runtime ones:
> each distro has to pick up a set of options and all
> users are stuck with it.
> So let's assume Ubuntu sets this and something is broken. How is a
> Fedora user to test this? Find out configure flags that Ubuntu uses
> and rebuild from source? It's hard to get people to triage bugs as it
> is.
> That's why changing behaviour in subtle ways depending on
> configure options is a very bad idea.
All it changes is a default machine type. I wouldn't classify that as
"very bad". Fortunately, our difference of opinion doesn't matter,
because the conclusion of the thread is "leave it to the distros".
> So a flag might be better, but if we want to be compatible
> with qemu-kvm, poking at argv[0] still better than configure.
That I do consider a genuinely bad idea.
Example of usage messed up by it: I symlink from my ~/bin to executables
in various build trees. If one of these programs changed behavior
depending on what it finds in argv[0], I'd have to know *exactly* how it
matches argv[0] to choose suitable names for my symlinks.
The name "qemu-kvm" is not a useful indicator of compatibility with the
qemu-kvm fork of QEMU anyway. There are programs out there calling
themselves qemu-kvm that aren't compatible with the qemu-kvm fork. And
there are programs out there that are compatible, but call themselves
something else.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 23.09.2014 08:06, Martin Kletzander wrote:
On Mon, Sep 22, 2014 at 04:50:33PM -0600, Jim Fehlig wrote:
Michal Privoznik wrote:
On 20.09.2014 01:36, Jim Fehlig wrote:
Martin Kletzander wrote:
Unfortunately I'm not very familiar with systemd files, but my guess
is that After=ntp-wait.service means it should be started after the
time is synchronized if and only if the ntp-wait.service unit is
enabled, otherwise it doesn't require it.
Yes, this is my understanding too.
And so is mine. The only concern I have is that syncing time on cold
boot of the host may take ages.
Yep, I have this concern too. So I dug a bit further and see that
ntp-wait (a perl script) scrapes the output of `ntpq -c "rv 0`, waiting
for leap_alarm to change to leap_none, leap_add_sec, or leap_del_sec.
On my test system, this took ~16min on cold boot :-(. ntp-wait.service
failed in the meantime, since it by default calls /usr/sbin/ntp-wait
with options to only wait 10min.
But on the other hand, it's better to start domains later and with
correct time than start asap with inaccurate time. ACK then,
Given the above observations, I'll wait to see if you change your mind.
What would you say to changing it to After=ntpdate.service? That way
it won't wait until the clock is synchronized, but it will be started
with proper time if ntpdate.service is set up to start in the default
runlevel (or is it target in systemd?). I think that's a compromise
that has no negative side-effects.
I wonder if we should be this specific or use time-sync.target:
time-sync.target
Services responsible for synchronizing the system clock from a
remote source (such as NTP client implementations) should pull in this
target and order themselves before it. All services where correct time
is essential should be ordered after this unit, but not pull it in.
systemd automatically adds dependencies of type After= for this target
unit to all SysV init script service units with an LSB header referring
to the "$time" facility.
http://www.freedesktop.org/software/systemd/man/systemd.special.html#time-sync.target
So I'd say: After=time-sync.target is what we are looking for.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
