Re: [libvirt] [PATCH v2] test: fix nwfilter tests following changes in virfirewall.c
On Sun, Dec 21, 2014 at 10:52:39AM -0500, Stefan Berger wrote: On 11/26/2014 11:53 AM, Eric Blake wrote: On 11/26/2014 09:26 AM, Stefan Berger wrote: Some of the nwfilter tests are now failing since --concurrent shows up in the ebtables command. To avoid this, implement a function preventing the probing for lock support in the eb/iptables tools and use it in the tests. Now that I've read Martin and Prerna's exchange, I'm wondering if we should instead make this override force the locking flags ON, and adjust the expected test output to expect the -w/--concurrent. Either this or the other patch should have made it into v1.2.11... I was under the impression that it did. As I said earlier, changing tests to always use concurrent is a long run, not a simple sed -i ... or something. And I don't feel like it's of a huge importance as I, for example, have around 3 other tests failing when building as root. 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 5/6] qemu: Leave cpuset.mems in parent cgroup alone
On Wed, Dec 17, 2014 at 04:59:30PM -0700, Eric Blake wrote: On 12/17/2014 08:06 AM, Martin Kletzander wrote: On Wed, Dec 17, 2014 at 12:00:36AM -0700, Eric Blake wrote: On 12/16/2014 11:51 PM, Eric Blake wrote: On 12/15/2014 12:58 AM, Martin Kletzander wrote: Instead of setting the value of cpuset.mems once when the domain starts and then re-calculating the value every time we need to change the child cgroup values, leave the cgroup alone and rather set the child data every time there is new cgroup created. We don't leave any task in the parent group anyway. This will ease both current and future code. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_cgroup.c | 67 -- src/qemu/qemu_driver.c | 59 +++- 2 files changed, 85 insertions(+), 41 deletions(-) This patch causes libvirtd to segfault on startup: More particularly, I'm doing an upgrade from older libvirtd to this version, while leaving a transient domain running that was started by the older libvirtd. Hope that helps you narrow in on the problem. Weird - At the time I made the report, I ran 'git bisect' and reliably reproduced the crash across multiple libvirtd restarts (a restart for each new build while trying to nail down the culprit commit), as long as I left that transient domain running. I tried that and it works for me. And I tried various domains, both heavily cgroup dependent and simple ones. But now that I've rebooted, and then proceeded to do incremental builds from both before and after the patch, I can't reproduce the crash. Although my formula for creating my transient domain was the same both yesterday and today, there may be some difference in the version of libvirtd that was running at the time I created the domain that then affected the XML affecting the libvirtd restarts. Reverting 86759e and af2a1f0 was sufficient to get me going again locally, but I'm not going to push my reversions until you've first had a chance to address the regression. #2 0x77405673 in virCgroupHasEmptyTasks (cgroup=0x0, controller=2) at util/vircgroup.c:3935 From this line it looks like priv-cgroup was not initialized. I did not add a check for that, so that may be the cause. I'll send a patch soon. But I wonder how did you manage to do that, is that a session libvirtd you restarted? Otherwise how come virCgroupNewDetectMachine() didn't fill the cgroup? I'm not spotting anything obvious why it wasn't initialized at the time I reproduced the crash, nor why I can't reproduce it now. Hopefully it's not a lurking time bomb. If it happens again, I'll definitely report it; but for now, without a reliable reproduction, it could easily have been something caused on my end (since it is my dev machine, it may have been caused by a half-baked patch on my end that I was testing at the time I created the transient domain, where using only upstream patches wouldn't see the issue). So for now, don't worry too hard if you can't find it either. There is a valid code path that allows the cgroup to be NULL, although not easy to reach. However, since it is valid, we added a patch that just skips the cgroup restoration for cgroup == NULL. If you happen to stumble upon a similar problem, I'd be happy to try dealing with it. 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 V3 1/5] Add public API virDomainSendSysrq
On 12/21/2014 10:15 PM, Chun Yan Liu wrote:
On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, John
Ferlan jfer...@redhat.com wrote:
On 12/19/2014 12:31 AM, Chun Yan Liu wrote:
On 12/18/2014 at 01:00 PM, in message
5492d008026600086...@soto.provo.novell.com, Chun Yan Liu
cy...@suse.com wrote:
On 12/17/2014 at 06:52 PM, in message
20141217105227.gq136...@orkuz.home,
Jiri Denemark jdene...@redhat.com wrote:
On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:
Add public API virDomainSendSysrq for sending SysRequest key.
Signed-off-by: Chunyan Liu cy...@suse.com
---
changes:
* add 'flags' to the new API
* change parameter from 'const char *key' to 'char key'
* change version number from 1.2.11 to 1.2.12
include/libvirt/libvirt-domain.h | 3 +++
src/driver-hypervisor.h | 4
src/libvirt-domain.c | 39
+++
src/libvirt_public.syms | 5 +
4 files changed, 51 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index baef32d..5f72850 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,
virDomainFSInfoPtr **info,
unsigned int flags);
+/* virDomainSendSysrq */
+int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags);
+
I think quite a few reviewers (Daniel, Eric, and I) agreed on using an
enum instead of char so that the API is more general.
Sorry, I missed this part. I'll update. One left question:
How about 'virsh sysrq' parameters? What would we expect users to pass?
Any thoughts on that?
libxl_send_sysrq
Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what
you had in mind for syntax previously - although it looks like:
virsh sysrq domain [key]
Thanks for reply. The syntax I'm used previously is:
#virsh sysrq domain key
key is required. It's just a letter, like 'h', 'c', etc. About which options
can we
have, on can refer to the results on guest through sysrq help. (that is, issue
'virsh sysrq domain h' and look at guest kernel message. I think on each
guest,
there must be 'h' option, it will print help message.)
h, c, etc. doesn't tell me enough about what to expect from the
perspective of this naive user... Passing 'h' via virsh to a driver
to return some help string that gets displayed where? Was there a
mechanism I missed to return and display that output? Do you have sample
output to show on a system with these changes applied?
Where if not provided key would be NULL, which doesn't look good for how
the code reads now.
As said above, key is required, it couldn't be NULL, otherwise, it will
report error.
While the check in virsh because VSH_OFLAG_REQ is set for key is good,
what if someone calls the API directly? You have no check there for
key being non null - it just gets passed along.
The description for key in virDomainSendSysrq is
still not sufficient to help me either:
+ * @key:SysRq key, like h, c, ...
What does 'h', 'c', ... mean? What are the options? What do they map to
functionality wise? I assume it's hypervisor dependent, but that's all
stuff you need to describe somewhere. I don't want to guess or go
searching for the answer through numerous search engine hits.
I can add more description on how one could get those options, but the way
I think is through 'sysrq help' and check guest message.
Looking at the enum Jirka proposed:
typedef enum {
VIR_DOMAIN_SYSRQ_REBOOT,
VIR_DOMAIN_SYSRQ_CRASH,
VIR_DOMAIN_SYSRQ_OOM_KILL,
VIR_DOMAIN_SYSRQ_SYNC,
...
} virDomainSysrqCommand;
It seems REBOOT would/could be the default. So if key wasn't provided
the incoming key would be 0 (zero)... If you didn't want a default,
then you'd have to force a style to be chosen. You're defining the API
so you show us how you want to handle that. Eventually, each hypervisor
would map that enum into a character. That is, you'll end up with a way
to map the enum to a letter for the types of sysrq's each hypervisor
could support. If a hypervisor doesn't support a specific type of sysrq,
then decide how to handle.
Anyway given the above enum list, I would think the virsh would be:
virsh sysrq domain reboot
virsh sysrq domain crash
virsh sysrq domain kill
virsh sysrq domain sync
...
OK. That's what I'm concerned and why I hesitated to change API parameter
from 'char key' to 'enum'. Personally I don't think this is a better user
interface and has risk to miss some functionality, since we don't know
which options those hypervisors can support.
If some other option is to be supported on some
Re: [libvirt] [PATCH v2 2/2] qemu: completely rework reference counting
My Coverity scan found two issues both FORWARD_NULL...
qemuDomainLookupByName
and
qemuMigrationPrepareAny
On 12/16/2014 05:15 AM, Martin Kletzander wrote:
There is one problem that causes various errors in the daemon. When
domain is waiting for a job, it is unlocked while waiting on the
condition. However, if that domain is for example transient and being
removed in another API (e.g. cancelling incoming migration), it get's
unref'd. If the first call, that was waiting, fails to get the job, it
unref's the domain object, and because it was the last reference, it
causes clearing of the whole domain object. However, when finishing the
call, the domain must be unlocked, but there is no way for the API to
know whether it was cleaned or not (unless there is some ugly temporary
variable, but let's scratch that).
The root cause is that our APIs don't ref the objects they are using and
all use the implicit reference that the object has when it is in the
domain list. That reference can be removed when the API is waiting for
a job. And because each domain doesn't do its ref'ing, it results in
the ugly checking of the return value of virObjectUnref() that we have
everywhere.
This patch changes qemuDomObjFromDomain() to ref the domain (using
virDomainObjListFindByUUIDRef()) and adds qemuDomObjEndAPI() which
should be the only function in which the return value of
virObjectUnref() is checked. This makes all reference counting
deterministic and makes the code a bit clearer.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
v2:
- Comments from Peter and Daniel on v1 implemented, rather not
listing them here as the list was pretty comprehensive and it would
make the reviewer focus on that.
src/qemu/THREADS.txt | 40 ++-
src/qemu/qemu_domain.c| 49 ++--
src/qemu/qemu_domain.h| 12 +-
src/qemu/qemu_driver.c| 708
--
src/qemu/qemu_migration.c | 111 +++-
src/qemu/qemu_migration.h | 10 +-
src/qemu/qemu_process.c | 77 ++---
7 files changed, 371 insertions(+), 636 deletions(-)
...snip...
@@ -1517,8 +1515,7 @@ static virDomainPtr
qemuDomainLookupByName(virConnectPtr conn,
if (dom) dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
Um... Did you mean the qemuDomObjEndAPI call here?
return dom;
}
...snip...
@@ -3099,12 +3089,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
* This prevents any other APIs being invoked while incoming
* migration is taking place.
*/
-if (!qemuMigrationJobContinue(vm)) {
-vm = NULL;
-virReportError(VIR_ERR_OPERATION_FAILED,
- %s, _(domain disappeared));
-goto cleanup;
-}
+qemuMigrationJobContinue(vm);
if (autoPort)
priv-migrationPort = port;
@@ -3115,16 +3100,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
VIR_FREE(xmlout);
VIR_FORCE_CLOSE(dataFD[0]);
VIR_FORCE_CLOSE(dataFD[1]);
-if (vm) {
-if (ret 0) {
-virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
-priv-nbdPort = 0;
-}
-if (ret = 0 || vm-persistent)
-virObjectUnlock(vm);
-else
-qemuDomainRemoveInactive(driver, vm);
+if (ret 0) {
+virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
We can get here with priv == NULL from numerous places...
+priv-nbdPort = 0;
+qemuDomainRemoveInactive(driver, vm);
}
+qemuDomObjEndAPI(vm);
if (event)
qemuDomainEventQueue(driver, event);
qemuMigrationCookieFree(mig);
@@ -3137,8 +3118,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0);
endjob:
-if (!qemuMigrationJobFinish(driver, vm))
-vm = NULL;
+qemuMigrationJobFinish(driver, vm);
goto cleanup;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Zanata xml config
Hi, The migration of the libvirt project translations to Zanata is now complete. Translators can start working now at https://fedora.zanata.org/project/view/libvirt To use Zanata, you need to 1. Register in https://fedora.zanata.org 2. If you are not the maintainer of your project yet, contact the admin to add you as maintainer. 3. Follow the instruction http://zanata.org/help/cli/cli-configuration/ 4. Place the attached zanata.xml in the root directory of repository 5. (Optional) integrate zanata push and zanata pull to your build scripts. If you have any questions , feel free to contact the Zanata dev team. Regards, -- Carlos A. Munoz Software Engineering Supervisor Engineering - Internationalization Red Hat ?xml version=1.0 encoding=UTF-8 standalone=yes? config xmlns=http://zanata.org/namespace/config/; urlhttps://fedora.zanata.org//url projectlibvirt/project project-versionmaster/project-version project-typegettext/project-type src-dir./src-dir trans-dir./trans-dir includespo/*.pot/includes locales localeaf/locale localesq/locale localeam/locale localear/locale localeas/locale localeast/locale localebal/locale localeeu/locale localebe/locale localebn/locale localebn-IN/locale localebrx/locale localebs/locale localebr/locale localebg/locale localeca/locale localezh-CN/locale localezh-HK/locale localezh-TW/locale localekw/locale localekw@kkcor/locale localekw@uccor/locale localekw-GB/locale localehr/locale localecs/locale localeda/locale localenl/locale localeen-GB/locale localeeo/locale localeet/locale localefi/locale localefr/locale localegl/locale localeka/locale localede/locale localede-CH/locale localeel/locale localegu/locale localehe/locale localehi/locale localehu/locale localeis/locale localeilo/locale localeid/locale localeia/locale localeit/locale localeja/locale localekn/locale localekk/locale localekm/locale localeky/locale localeko/locale localelv/locale localelt/locale localends/locale localemk/locale localemai/locale localems/locale localeml/locale localemr/locale localemn/locale localene/locale localenso/locale localenb/locale localenn/locale localeor/locale localefa/locale localepl/locale localept/locale localept-BR/locale localepa/locale localero/locale localeru/locale localesr/locale localesr@latin/locale localesi/locale localesk/locale localesl/locale localees/locale localesv/locale localetg/locale localeta/locale localete/locale localeth/locale localebo/locale localetr/locale localetw/locale localeuk/locale localeur/locale localevi/locale localecy/locale localezu/locale localewba/locale /locales rules/ /config -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled
On 12/21/2014 08:57 PM, Chen Hanxiao wrote: s/namespce/namespace/ in the subject line If we enabled user ns and provided a uid/gid map, we do not need to mount /proc, /sys as readonly. Leave it to kernel for protection. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 6 ++ 1 file changed, 6 insertions(+) I'll leave the actual patch review to someone more familiar with LXC namespace setups -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] test: fix nwfilter tests following changes in virfirewall.c
On 12/22/2014 04:37 AM, Martin Kletzander wrote: On Sun, Dec 21, 2014 at 10:52:39AM -0500, Stefan Berger wrote: On 11/26/2014 11:53 AM, Eric Blake wrote: On 11/26/2014 09:26 AM, Stefan Berger wrote: Some of the nwfilter tests are now failing since --concurrent shows up in the ebtables command. To avoid this, implement a function preventing the probing for lock support in the eb/iptables tools and use it in the tests. Now that I've read Martin and Prerna's exchange, I'm wondering if we should instead make this override force the locking flags ON, and adjust the expected test output to expect the -w/--concurrent. Either this or the other patch should have made it into v1.2.11... I was under the impression that it did. As I said earlier, changing tests to always use concurrent is a long run, not a simple sed -i ... or something. And I don't feel like it's of a huge importance as I, for example, have around 3 other tests failing when building as root. On Fedora 20 I get 3 skips and 2 failures. The 2 failures are solved by the posted patch. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] test: fix nwfilter tests following changes in virfirewall.c
On Mon, Dec 22, 2014 at 10:26:01AM -0500, Stefan Berger wrote: On 12/22/2014 04:37 AM, Martin Kletzander wrote: On Sun, Dec 21, 2014 at 10:52:39AM -0500, Stefan Berger wrote: On 11/26/2014 11:53 AM, Eric Blake wrote: On 11/26/2014 09:26 AM, Stefan Berger wrote: Some of the nwfilter tests are now failing since --concurrent shows up in the ebtables command. To avoid this, implement a function preventing the probing for lock support in the eb/iptables tools and use it in the tests. Now that I've read Martin and Prerna's exchange, I'm wondering if we should instead make this override force the locking flags ON, and adjust the expected test output to expect the -w/--concurrent. Either this or the other patch should have made it into v1.2.11... I was under the impression that it did. As I said earlier, changing tests to always use concurrent is a long run, not a simple sed -i ... or something. And I don't feel like it's of a huge importance as I, for example, have around 3 other tests failing when building as root. On Fedora 20 I get 3 skips and 2 failures. The 2 failures are solved by the posted patch. ACK for this version if it fixes all problems on F20. I need to check why it still fails for me though, but that's nothing that should concern you. 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 v2 2/2] qemu: completely rework reference counting
On Mon, Dec 22, 2014 at 07:27:29AM -0500, John Ferlan wrote:
My Coverity scan found two issues both FORWARD_NULL...
qemuDomainLookupByName
and
qemuMigrationPrepareAny
On 12/16/2014 05:15 AM, Martin Kletzander wrote:
There is one problem that causes various errors in the daemon. When
domain is waiting for a job, it is unlocked while waiting on the
condition. However, if that domain is for example transient and being
removed in another API (e.g. cancelling incoming migration), it get's
unref'd. If the first call, that was waiting, fails to get the job, it
unref's the domain object, and because it was the last reference, it
causes clearing of the whole domain object. However, when finishing the
call, the domain must be unlocked, but there is no way for the API to
know whether it was cleaned or not (unless there is some ugly temporary
variable, but let's scratch that).
The root cause is that our APIs don't ref the objects they are using and
all use the implicit reference that the object has when it is in the
domain list. That reference can be removed when the API is waiting for
a job. And because each domain doesn't do its ref'ing, it results in
the ugly checking of the return value of virObjectUnref() that we have
everywhere.
This patch changes qemuDomObjFromDomain() to ref the domain (using
virDomainObjListFindByUUIDRef()) and adds qemuDomObjEndAPI() which
should be the only function in which the return value of
virObjectUnref() is checked. This makes all reference counting
deterministic and makes the code a bit clearer.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
v2:
- Comments from Peter and Daniel on v1 implemented, rather not
listing them here as the list was pretty comprehensive and it would
make the reviewer focus on that.
src/qemu/THREADS.txt | 40 ++-
src/qemu/qemu_domain.c| 49 ++--
src/qemu/qemu_domain.h| 12 +-
src/qemu/qemu_driver.c| 708 --
src/qemu/qemu_migration.c | 111 +++-
src/qemu/qemu_migration.h | 10 +-
src/qemu/qemu_process.c | 77 ++---
7 files changed, 371 insertions(+), 636 deletions(-)
...snip...
@@ -1517,8 +1515,7 @@ static virDomainPtr qemuDomainLookupByName(virConnectPtr
conn,
if (dom) dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
Um... Did you mean the qemuDomObjEndAPI call here?
No, because the vm was gotten using virDomainObjListFindByName() which
does not ref it. But instead it should be kept as it was.
return dom;
}
...snip...
@@ -3099,12 +3089,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
* This prevents any other APIs being invoked while incoming
* migration is taking place.
*/
-if (!qemuMigrationJobContinue(vm)) {
-vm = NULL;
-virReportError(VIR_ERR_OPERATION_FAILED,
- %s, _(domain disappeared));
-goto cleanup;
-}
+qemuMigrationJobContinue(vm);
if (autoPort)
priv-migrationPort = port;
@@ -3115,16 +3100,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
VIR_FREE(xmlout);
VIR_FORCE_CLOSE(dataFD[0]);
VIR_FORCE_CLOSE(dataFD[1]);
-if (vm) {
-if (ret 0) {
-virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
-priv-nbdPort = 0;
-}
-if (ret = 0 || vm-persistent)
-virObjectUnlock(vm);
-else
-qemuDomainRemoveInactive(driver, vm);
+if (ret 0) {
+virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
We can get here with priv == NULL from numerous places...
Yes, good point. The following diff should suffice for both issues,
right? If you agree, do you want me to send a patch or just push it?
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 673d8a6..73a825d 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -1516,7 +1516,8 @@ static virDomainPtr qemuDomainLookupByName(virConnectPtr
conn,
if (dom) dom-id = vm-def-id;
cleanup:
-virObjectUnlock(vm);
+if (vm)
+virObjectUnlock(vm);
return dom;
}
diff --git i/src/qemu/qemu_migration.c w/src/qemu/qemu_migration.c
index 1db6630..77e0b35 100644
--- i/src/qemu/qemu_migration.c
+++ w/src/qemu/qemu_migration.c
@@ -3101,7 +3101,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
VIR_FREE(xmlout);
VIR_FORCE_CLOSE(dataFD[0]);
VIR_FORCE_CLOSE(dataFD[1]);
-if (ret 0) {
+if (ret 0 priv) {
+/* priv is set right after vm is added to the list of domains
+ * and there is no 'goto cleanup;' in the middle of those */
virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
priv-nbdPort = 0;
qemuDomainRemoveInactive(driver, vm);
--
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [openstack-dev] [nova] - 'nova reboot' causes console-log truncated
On 11/14/14 2:02 AM, Daniel P. Berrange wrote: On Thu, Nov 13, 2014 at 01:55:06PM -0800, Surojit Pathak wrote: Hi all, [Issue observed] If we issue 'nova reboot server', we get to have the console output of the latest bootup of the server only. The console output of the previous boot for the same server vanishes due to truncation[1]. If we do reboot from within the VM instance [ #sudo reboot ], or reboot the instance with 'virsh reboot instance' the behavior is not the same, where the console.log keeps increasing, with the new output being appended. This loss of history makes some debugging scenario difficult due to lack of information being available. Please point me to any solution/blueprint for this issue, if already planned. Otherwise, please comment on my analysis and proposals as solution, below - [Analysis] Nova's libvirt driver on compute node tries to do a graceful restart of the server instance, by attempting a soft_reboot first. If soft_reboot fails, it attempts a hard_reboot. As part of soft_reboot, it brings down the instance by calling shutdown(), and then calls createWithFlags() to bring this up. Because of this, qemu-kvm process for the instance gets terminated and new process is launched. In QEMU, the chardev file is opened with O_TRUNC, and thus we lose the previous content of the console.log file. On the other-hand, during 'virsh reboot instance', the same qemu-kvm process continues, and libvirt actually does a qemuDomainSetFakeReboot(). Thus the same file continues capturing the new console output as a continuation into the same file. Nova and libvirt have support for issuing a graceful reboot via the QEMU guest agent. So if you make sure that is installed, and tell Nova to use it, then Nova won't have to stop recreate the QEMU process and thus won't have the problem of overwriting the logs. Hi Daniel, Having GA to do graceful restart is nice option. But if it were to just preserve the same console file, even 'virsh reboot' achieves the purpose. As I explained in my original analysis, Nova seems to have not taken the path, as it does not want to have a false positive, where the GA does not respond or 'virDomain.reboot' fails later and the domain is not really restarted. [ CC-ed vish, author of nova http://tripsgrips.corp.gq1.yahoo.com:8080/source/xref/nova/nova//virt http://tripsgrips.corp.gq1.yahoo.com:8080/source/xref/nova/nova/virt//libvirt http://tripsgrips.corp.gq1.yahoo.com:8080/source/xref/nova/nova/virt/libvirt//driver.py http://tripsgrips.corp.gq1.yahoo.com:8080/source/xref/nova/nova/virt/libvirt/driver.py ] IMHO, QEMU should preserve the console-log file for a given domain, if it exists, by not opening with O_TRUNC option, instead opening with O_APPEND. I would like to draw a comparison of a real computer to which we might be connected over serial console, and the box gets powered down and up with external button press, and we do not lose the console history, if connected. And that's what is the experience console-log intends to provide. If you think, this is agreeable, please let me know, I will send the patch to qemu-devel@. -- Regards, SURO -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Add tests to xmconfigtest
Chunyan Liu wrote: Hi Chunyan, Thanks for the fix, and the test! But I have a few questions regarding the latter... Add tests to testing HVM default features (pae, acpi, apic) conversion from xm config to libvirt xml and from libvirt xml to xm config. Signed-off-by: Chunyan Liu cy...@suse.com --- .../xmconfigdata/test-fullvirt-default-feature.cfg | 23 +++ .../test-fullvirt-default-feature.cfg.out | 26 .../xmconfigdata/test-fullvirt-default-feature.xml | 48 ++ tests/xmconfigtest.c | 9 +++- 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg.out create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.xml diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg b/tests/xmconfigdata/test-fullvirt-default-feature.cfg new file mode 100644 index 000..5ce234f --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg Why is this file needed? @@ -0,0 +1,23 @@ +name = XenGuest2 +uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809 +maxmem = 579 +memory = 394 +vcpus = 1 +builder = hvm +kernel = /usr/lib/xen/boot/hvmloader +boot = d +hpet = 1 +localtime = 0 +on_poweroff = destroy +on_reboot = restart +on_crash = restart +device_model = /usr/lib/xen/bin/qemu-dm +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = 127.0.0.1 +vncpasswd = 123poi +vif = [ mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu ] +parallel = none +serial = none +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, file:/root/boot.iso,hdc:cdrom,r ] diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out new file mode 100644 index 000..97a9827 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out IMO, this file should be renamed to 'test-fullvirt-default-feature.cfg'. @@ -0,0 +1,26 @@ +name = XenGuest2 +uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809 +maxmem = 579 +memory = 394 +vcpus = 1 +builder = hvm +kernel = /usr/lib/xen/boot/hvmloader +boot = d +pae = 1 +acpi = 1 +apic = 1 +hpet = 1 +localtime = 0 +on_poweroff = destroy +on_reboot = restart +on_crash = restart +device_model = /usr/lib/xen/bin/qemu-dm +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = 127.0.0.1 +vncpasswd = 123poi +vif = [ mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu ] +parallel = none +serial = none +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, file:/root/boot.iso,hdc:cdrom,r ] diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.xml b/tests/xmconfigdata/test-fullvirt-default-feature.xml new file mode 100644 index 000..57a6531 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.xml @@ -0,0 +1,48 @@ +domain type='xen' + nameXenGuest2/name + uuidc7a5fdb2-cdaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'592896/memory + currentMemory unit='KiB'403456/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='xenfv'hvm/type +loader type='rom'/usr/lib/xen/boot/hvmloader/loader +boot dev='cdrom'/ + /os + features +acpi/ +apic/ +pae/ + /features + clock offset='utc' adjustment='reset' +timer name='hpet' present='yes'/ + /clock + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashrestart/on_crash + devices +emulator/usr/lib/xen/bin/qemu-dm/emulator +disk type='block' device='disk' + driver name='phy'/ + source dev='/dev/HostVG/XenGuest2'/ + target dev='hda' bus='ide'/ +/disk +disk type='file' device='cdrom' + driver name='file'/ + source file='/root/boot.iso'/ + target dev='hdc' bus='ide'/ + readonly/ +/disk +interface type='bridge' + mac address='00:16:3e:66:92:9c'/ + source bridge='xenbr1'/ + script path='vif-bridge'/ + model type='e1000'/ +/interface +input type='mouse' bus='ps2'/ +input type='keyboard' bus='ps2'/ +graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi' + listen type='address' address='127.0.0.1'/ +/graphics + /devices +/domain diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index 0c6f803..b0b7b30 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -176,21 +176,26 @@ testCompareHelper(const void *data) const struct testInfo *info = data; char *xml = NULL; char *cfg = NULL; +char *cfgout = NULL; if (virAsprintf(xml, %s/xmconfigdata/test-%s.xml, abs_srcdir, info-name) 0 || virAsprintf(cfg, %s/xmconfigdata/test-%s.cfg, +abs_srcdir, info-name) 0 || +
Re: [libvirt] [PATCH 2/2] Add tests to xmconfigtest
On 12/23/2014 at 09:42 AM, in message 5498c888.6000...@suse.com, Jim Fehlig jfeh...@suse.com wrote: Chunyan Liu wrote: Hi Chunyan, Thanks for the fix, and the test! But I have a few questions regarding the latter... Add tests to testing HVM default features (pae, acpi, apic) conversion from xm config to libvirt xml and from libvirt xml to xm config. Signed-off-by: Chunyan Liu cy...@suse.com --- .../xmconfigdata/test-fullvirt-default-feature.cfg | 23 +++ .../test-fullvirt-default-feature.cfg.out | 26 .../xmconfigdata/test-fullvirt-default-feature.xml | 48 ++ tests/xmconfigtest.c | 9 +++- 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg.out create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.xml diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg b/tests/xmconfigdata/test-fullvirt-default-feature.cfg new file mode 100644 index 000..5ce234f --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg Why is this file needed? Here we are testing default value conversion. That is: if there is no apci/pae/apic specified in xm config, after conversion, libvirt xml should include: features apic/ acpi/ pae/ /features So, from xm config - xml, the cfg file should be like this. @@ -0,0 +1,23 @@ +name = XenGuest2 +uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809 +maxmem = 579 +memory = 394 +vcpus = 1 +builder = hvm +kernel = /usr/lib/xen/boot/hvmloader +boot = d +hpet = 1 +localtime = 0 +on_poweroff = destroy +on_reboot = restart +on_crash = restart +device_model = /usr/lib/xen/bin/qemu-dm +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = 127.0.0.1 +vncpasswd = 123poi +vif = [ mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioem u ] +parallel = none +serial = none +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, file:/root/boot.iso,hdc:cdrom,r ] diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out new file mode 100644 index 000..97a9827 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out IMO, this file should be renamed to 'test-fullvirt-default-feature.cfg'. And from xml - xm config, if in xml there is: features apic/ acpi/ pae/ /features Then after conversion, in xm config out file there will be explicitly: acpi=1 apic=1 pae=1 So, thlis is a little different from test-fullvirt-default-feature.cfg. @@ -0,0 +1,26 @@ +name = XenGuest2 +uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809 +maxmem = 579 +memory = 394 +vcpus = 1 +builder = hvm +kernel = /usr/lib/xen/boot/hvmloader +boot = d +pae = 1 +acpi = 1 +apic = 1 +hpet = 1 +localtime = 0 +on_poweroff = destroy +on_reboot = restart +on_crash = restart +device_model = /usr/lib/xen/bin/qemu-dm +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = 127.0.0.1 +vncpasswd = 123poi +vif = [ mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioem u ] +parallel = none +serial = none +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, file:/root/boot.iso,hdc:cdrom,r ] diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.xml b/tests/xmconfigdata/test-fullvirt-default-feature.xml new file mode 100644 index 000..57a6531 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.xml @@ -0,0 +1,48 @@ +domain type='xen' + nameXenGuest2/name + uuidc7a5fdb2-cdaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'592896/memory + currentMemory unit='KiB'403456/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='xenfv'hvm/type +loader type='rom'/usr/lib/xen/boot/hvmloader/loader +boot dev='cdrom'/ + /os + features +acpi/ +apic/ +pae/ + /features + clock offset='utc' adjustment='reset' +timer name='hpet' present='yes'/ + /clock + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashrestart/on_crash + devices +emulator/usr/lib/xen/bin/qemu-dm/emulator +disk type='block' device='disk' + driver name='phy'/ + source dev='/dev/HostVG/XenGuest2'/ + target dev='hda' bus='ide'/ +/disk +disk type='file' device='cdrom' + driver name='file'/ + source file='/root/boot.iso'/ + target dev='hdc' bus='ide'/ + readonly/ +/disk +interface type='bridge' + mac address='00:16:3e:66:92:9c'/ + source
Re: [libvirt] [PATCH V3 1/5] Add public API virDomainSendSysrq
On 12/22/2014 at 08:17 PM, in message 54980bf2.1060...@redhat.com, John
Ferlan jfer...@redhat.com wrote:
On 12/21/2014 10:15 PM, Chun Yan Liu wrote:
On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com,
John
Ferlan jfer...@redhat.com wrote:
On 12/19/2014 12:31 AM, Chun Yan Liu wrote:
On 12/18/2014 at 01:00 PM, in message
5492d008026600086...@soto.provo.novell.com, Chun Yan Liu
cy...@suse.com wrote:
On 12/17/2014 at 06:52 PM, in message
20141217105227.gq136...@orkuz.home,
Jiri Denemark jdene...@redhat.com wrote:
On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:
Add public API virDomainSendSysrq for sending SysRequest key.
Signed-off-by: Chunyan Liu cy...@suse.com
---
changes:
* add 'flags' to the new API
* change parameter from 'const char *key' to 'char key'
* change version number from 1.2.11 to 1.2.12
include/libvirt/libvirt-domain.h | 3 +++
src/driver-hypervisor.h | 4
src/libvirt-domain.c | 39
+++
src/libvirt_public.syms | 5 +
4 files changed, 51 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index baef32d..5f72850 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,
virDomainFSInfoPtr **info,
unsigned int flags);
+/* virDomainSendSysrq */
+int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int
flags);
+
I think quite a few reviewers (Daniel, Eric, and I) agreed on using an
enum instead of char so that the API is more general.
Sorry, I missed this part. I'll update. One left question:
How about 'virsh sysrq' parameters? What would we expect users to pass?
Any thoughts on that?
libxl_send_sysrq
Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what
you had in mind for syntax previously - although it looks like:
virsh sysrq domain [key]
Thanks for reply. The syntax I'm used previously is:
#virsh sysrq domain key
key is required. It's just a letter, like 'h', 'c', etc. About which
options can we
have, on can refer to the results on guest through sysrq help. (that is,
issue
'virsh sysrq domain h' and look at guest kernel message. I think on each
guest,
there must be 'h' option, it will print help message.)
h, c, etc. doesn't tell me enough about what to expect from the
perspective of this naive user... Passing 'h' via virsh to a driver
to return some help string that gets displayed where?
Guest kernel message if guest is Linux. xen/libxl just passes the key
blindly to guest kernel, so to pass 'h' to guest kernel, it just like one issue:
#echo 'h' /proc/sysrq-trigger
in a Linux guest, you will see in /var/log/messages:
SysRq : HELP : loglevel(0-9) reboot(b) crash(c) terminate-all-tasks(e)
memory-full-oom-kill(f) kill-all-tasks(i) thaw-filesystems(j)
sak(k) show-backtrace-all-active-cpus(l) show-memory-usage(m)
nice-all-RT-tasks(n) poweroff(o) show-registers(p) show-all-timers(q)
unraw(r) sync(s) show-task-states(t) unmount(u) force-fb(V)
show-blocked-tasks(w) dump-ftrace-buffer(z)
Was there a
mechanism I missed to return and display that output? Do you have sample
output to show on a system with these changes applied?
I don't know how if any other hypervisor behaves differently, for xen/libxl,
they just send sysrq key to guest blindly if I understand correctly. So, which
letter
is corresponding to which option is all the same with guest sysrq key
definition,
in other words, it depends on guest sysrq key definition.
Where if not provided key would be NULL, which doesn't look good for how
the code reads now.
As said above, key is required, it couldn't be NULL, otherwise, it will
report error.
While the check in virsh because VSH_OFLAG_REQ is set for key is good,
what if someone calls the API directly? You have no check there for
key being non null - it just gets passed along.
The description for key in virDomainSendSysrq is
still not sufficient to help me either:
+ * @key:SysRq key, like h, c, ...
What does 'h', 'c', ... mean? What are the options? What do they map to
functionality wise? I assume it's hypervisor dependent, but that's all
stuff you need to describe somewhere. I don't want to guess or go
searching for the answer through numerous search engine hits.
I can add more description on how one could get those options, but the way
I think is through 'sysrq help' and check guest message.
Looking at the enum Jirka proposed:
typedef enum {
Re: [libvirt] [PATCH 2/2] Add tests to xmconfigtest
Chun Yan Liu wrote: On 12/23/2014 at 09:42 AM, in message 5498c888.6000...@suse.com, Jim Fehlig jfeh...@suse.com wrote: Chunyan Liu wrote: Hi Chunyan, Thanks for the fix, and the test! But I have a few questions regarding the latter... Add tests to testing HVM default features (pae, acpi, apic) conversion from xm config to libvirt xml and from libvirt xml to xm config. Signed-off-by: Chunyan Liu cy...@suse.com --- .../xmconfigdata/test-fullvirt-default-feature.cfg | 23 +++ .../test-fullvirt-default-feature.cfg.out | 26 .../xmconfigdata/test-fullvirt-default-feature.xml | 48 ++ tests/xmconfigtest.c | 9 +++- 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg.out create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.xml diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg b/tests/xmconfigdata/test-fullvirt-default-feature.cfg new file mode 100644 index 000..5ce234f --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg Why is this file needed? Here we are testing default value conversion. That is: if there is no apci/pae/apic specified in xm config, after conversion, libvirt xml should include: features apic/ acpi/ pae/ /features Ah, ok. Agreed that this test is missing. So, from xm config - xml, the cfg file should be like this. @@ -0,0 +1,23 @@ +name = XenGuest2 +uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809 +maxmem = 579 +memory = 394 +vcpus = 1 +builder = hvm +kernel = /usr/lib/xen/boot/hvmloader +boot = d +hpet = 1 +localtime = 0 +on_poweroff = destroy +on_reboot = restart +on_crash = restart +device_model = /usr/lib/xen/bin/qemu-dm +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = 127.0.0.1 +vncpasswd = 123poi +vif = [ mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioem u ] +parallel = none +serial = none +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, file:/root/boot.iso,hdc:cdrom,r ] diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out new file mode 100644 index 000..97a9827 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out IMO, this file should be renamed to 'test-fullvirt-default-feature.cfg'. And from xml - xm config, if in xml there is: features apic/ acpi/ pae/ /features Then after conversion, in xm config out file there will be explicitly: acpi=1 apic=1 pae=1 So, thlis is a little different from test-fullvirt-default-feature.cfg. This is actually tested in all of the other test-fullvirt-* tests. I don't think we need an explicit test for it. @@ -0,0 +1,26 @@ +name = XenGuest2 +uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809 +maxmem = 579 +memory = 394 +vcpus = 1 +builder = hvm +kernel = /usr/lib/xen/boot/hvmloader +boot = d +pae = 1 +acpi = 1 +apic = 1 +hpet = 1 +localtime = 0 +on_poweroff = destroy +on_reboot = restart +on_crash = restart +device_model = /usr/lib/xen/bin/qemu-dm +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = 127.0.0.1 +vncpasswd = 123poi +vif = [ mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioem u ] +parallel = none +serial = none +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, file:/root/boot.iso,hdc:cdrom,r ] diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.xml b/tests/xmconfigdata/test-fullvirt-default-feature.xml new file mode 100644 index 000..57a6531 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.xml @@ -0,0 +1,48 @@ +domain type='xen' + nameXenGuest2/name + uuidc7a5fdb2-cdaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'592896/memory + currentMemory unit='KiB'403456/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='xenfv'hvm/type +loader type='rom'/usr/lib/xen/boot/hvmloader/loader +boot dev='cdrom'/ + /os + features +acpi/ +apic/ +pae/ + /features + clock offset='utc' adjustment='reset' +timer name='hpet' present='yes'/ + /clock + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashrestart/on_crash + devices +emulator/usr/lib/xen/bin/qemu-dm/emulator +disk type='block' device='disk' + driver name='phy'/ + source dev='/dev/HostVG/XenGuest2'/ + target dev='hda' bus='ide'/ +
Re: [libvirt] [openstack-dev] [nova] - 'nova reboot' causes console-log truncated
On 12/22/14 5:04 PM, Tony Breeds wrote: On Mon, Dec 22, 2014 at 04:36:02PM -0800, Surojit Pathak wrote: Hi Daniel, Having GA to do graceful restart is nice option. But if it were to just preserve the same console file, even 'virsh reboot' achieves the purpose. As I explained in my original analysis, Nova seems to have not taken the path, as it does not want to have a false positive, where the GA does not respond or 'virDomain.reboot' fails later and the domain is not really restarted. [ CC-ed vish, author of nova IMHO, QEMU should preserve the console-log file for a given domain, if it exists, by not opening with O_TRUNC option, instead opening with O_APPEND. I would like to draw a comparison of a real computer to which we might be connected over serial console, and the box gets powered down and up with external button press, and we do not lose the console history, if connected. And that's what is the experience console-log intends to provide. If you think, this is agreeable, please let me know, I will send the patch to qemu-devel@. The issue is more complex than just removing the O_TRUNC from the open() flags. I havd a proposal that will (almost by accident) fix this in qemu by allowing console log files to be rotated. I'm also waorking on a similar feature in libvirt. I think the tl;dr: is that this /shoudl/ be fixed in kilo with a 'modern' libvirt. Hi Tony, Can you please share some details of the effort, in terms of reference? Yours Tony. ___ OpenStack-dev mailing list openstack-...@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Regards, SURO -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Add tests to xmconfigtest
On 12/23/2014 at 11:13 AM, in message 5498ddcc.2020...@suse.com, Jim Fehlig jfeh...@suse.com wrote: Chun Yan Liu wrote: On 12/23/2014 at 09:42 AM, in message 5498c888.6000...@suse.com, Jim Fehlig jfeh...@suse.com wrote: Chunyan Liu wrote: Hi Chunyan, Thanks for the fix, and the test! But I have a few questions regarding the latter... Add tests to testing HVM default features (pae, acpi, apic) conversion from xm config to libvirt xml and from libvirt xml to xm config. Signed-off-by: Chunyan Liu cy...@suse.com --- .../xmconfigdata/test-fullvirt-default-feature.cfg | 23 +++ .../test-fullvirt-default-feature.cfg.out | 26 .../xmconfigdata/test-fullvirt-default-feature.xml | 48 ++ tests/xmconfigtest.c | 9 +++- 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg.out create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.xml diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg b/tests/xmconfigdata/test-fullvirt-default-feature.cfg new file mode 100644 index 000..5ce234f --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg Why is this file needed? Here we are testing default value conversion. That is: if there is no apci/pae/apic specified in xm config, after conversion, libvirt xml should include: features apic/ acpi/ pae/ /features Ah, ok. Agreed that this test is missing. So, from xm config - xml, the cfg file should be like this. @@ -0,0 +1,23 @@ +name = XenGuest2 +uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809 +maxmem = 579 +memory = 394 +vcpus = 1 +builder = hvm +kernel = /usr/lib/xen/boot/hvmloader +boot = d +hpet = 1 +localtime = 0 +on_poweroff = destroy +on_reboot = restart +on_crash = restart +device_model = /usr/lib/xen/bin/qemu-dm +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = 127.0.0.1 +vncpasswd = 123poi +vif = [ mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioem u ] +parallel = none +serial = none +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, file:/root/boot.iso,hdc:cdrom,r ] diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out new file mode 100644 index 000..97a9827 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out IMO, this file should be renamed to 'test-fullvirt-default-feature.cfg'. And from xml - xm config, if in xml there is: features apic/ acpi/ pae/ /features Then after conversion, in xm config out file there will be explicitly: acpi=1 apic=1 pae=1 So, thlis is a little different from test-fullvirt-default-feature.cfg. This is actually tested in all of the other test-fullvirt-* tests. I don't think we need an explicit test for it. @@ -0,0 +1,26 @@ +name = XenGuest2 +uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809 +maxmem = 579 +memory = 394 +vcpus = 1 +builder = hvm +kernel = /usr/lib/xen/boot/hvmloader +boot = d +pae = 1 +acpi = 1 +apic = 1 +hpet = 1 +localtime = 0 +on_poweroff = destroy +on_reboot = restart +on_crash = restart +device_model = /usr/lib/xen/bin/qemu-dm +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = 127.0.0.1 +vncpasswd = 123poi +vif = [ mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioem u ] +parallel = none +serial = none +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, file:/root/boot.iso,hdc:cdrom,r ] diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.xml b/tests/xmconfigdata/test-fullvirt-default-feature.xml new file mode 100644 index 000..57a6531 --- /dev/null +++ b/tests/xmconfigdata/test-fullvirt-default-feature.xml @@ -0,0 +1,48 @@ +domain type='xen' + nameXenGuest2/name + uuidc7a5fdb2-cdaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'592896/memory + currentMemory unit='KiB'403456/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='xenfv'hvm/type +loader type='rom'/usr/lib/xen/boot/hvmloader/loader +boot dev='cdrom'/ + /os +
Re: [libvirt] [PATCH v2 2/2] qemu: completely rework reference counting
On 12/22/2014 04:02 PM, Martin Kletzander wrote:
On Mon, Dec 22, 2014 at 07:27:29AM -0500, John Ferlan wrote:
My Coverity scan found two issues both FORWARD_NULL...
qemuDomainLookupByName
and
qemuMigrationPrepareAny
On 12/16/2014 05:15 AM, Martin Kletzander wrote:
There is one problem that causes various errors in the daemon. When
domain is waiting for a job, it is unlocked while waiting on the
condition. However, if that domain is for example transient and being
removed in another API (e.g. cancelling incoming migration), it get's
unref'd. If the first call, that was waiting, fails to get the job, it
unref's the domain object, and because it was the last reference, it
causes clearing of the whole domain object. However, when finishing the
call, the domain must be unlocked, but there is no way for the API to
know whether it was cleaned or not (unless there is some ugly temporary
variable, but let's scratch that).
The root cause is that our APIs don't ref the objects they are using and
all use the implicit reference that the object has when it is in the
domain list. That reference can be removed when the API is waiting for
a job. And because each domain doesn't do its ref'ing, it results in
the ugly checking of the return value of virObjectUnref() that we have
everywhere.
This patch changes qemuDomObjFromDomain() to ref the domain (using
virDomainObjListFindByUUIDRef()) and adds qemuDomObjEndAPI() which
should be the only function in which the return value of
virObjectUnref() is checked. This makes all reference counting
deterministic and makes the code a bit clearer.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
v2:
- Comments from Peter and Daniel on v1 implemented, rather not
listing them here as the list was pretty comprehensive and it would
make the reviewer focus on that.
src/qemu/THREADS.txt | 40 ++-
src/qemu/qemu_domain.c| 49 ++--
src/qemu/qemu_domain.h| 12 +-
src/qemu/qemu_driver.c| 708
--
src/qemu/qemu_migration.c | 111 +++-
src/qemu/qemu_migration.h | 10 +-
src/qemu/qemu_process.c | 77 ++---
7 files changed, 371 insertions(+), 636 deletions(-)
...snip...
@@ -1517,8 +1515,7 @@ static virDomainPtr
qemuDomainLookupByName(virConnectPtr conn,
if (dom) dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
Um... Did you mean the qemuDomObjEndAPI call here?
No, because the vm was gotten using virDomainObjListFindByName() which
does not ref it. But instead it should be kept as it was.
return dom;
}
...snip...
@@ -3099,12 +3089,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
* This prevents any other APIs being invoked while incoming
* migration is taking place.
*/
-if (!qemuMigrationJobContinue(vm)) {
-vm = NULL;
-virReportError(VIR_ERR_OPERATION_FAILED,
- %s, _(domain disappeared));
-goto cleanup;
-}
+qemuMigrationJobContinue(vm);
if (autoPort)
priv-migrationPort = port;
@@ -3115,16 +3100,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
VIR_FREE(xmlout);
VIR_FORCE_CLOSE(dataFD[0]);
VIR_FORCE_CLOSE(dataFD[1]);
-if (vm) {
-if (ret 0) {
-virPortAllocatorRelease(driver-migrationPorts,
priv-nbdPort);
-priv-nbdPort = 0;
-}
-if (ret = 0 || vm-persistent)
-virObjectUnlock(vm);
-else
-qemuDomainRemoveInactive(driver, vm);
+if (ret 0) {
+virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
We can get here with priv == NULL from numerous places...
Yes, good point. The following diff should suffice for both issues,
right? If you agree, do you want me to send a patch or just push it?
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 673d8a6..73a825d 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -1516,7 +1516,8 @@ static virDomainPtr
qemuDomainLookupByName(virConnectPtr conn,
if (dom) dom-id = vm-def-id;
cleanup:
-virObjectUnlock(vm);
+if (vm)
+virObjectUnlock(vm);
return dom;
}
diff --git i/src/qemu/qemu_migration.c w/src/qemu/qemu_migration.c
index 1db6630..77e0b35 100644
--- i/src/qemu/qemu_migration.c
+++ w/src/qemu/qemu_migration.c
@@ -3101,7 +3101,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
VIR_FREE(xmlout);
VIR_FORCE_CLOSE(dataFD[0]);
VIR_FORCE_CLOSE(dataFD[1]);
-if (ret 0) {
+if (ret 0 priv) {
+/* priv is set right after vm is added to the list of domains
+ * and there is no 'goto cleanup;' in the middle of those */
virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
priv-nbdPort = 0;
qemuDomainRemoveInactive(driver, vm);
Makes
Re: [libvirt] [PATCH V3 1/5] Add public API virDomainSendSysrq
On 12/22/2014 09:55 PM, Chun Yan Liu wrote: On 12/22/2014 at 08:17 PM, in message 54980bf2.1060...@redhat.com, John Ferlan jfer...@redhat.com wrote: On 12/21/2014 10:15 PM, Chun Yan Liu wrote: On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, John Ferlan jfer...@redhat.com wrote: On 12/19/2014 12:31 AM, Chun Yan Liu wrote: On 12/18/2014 at 01:00 PM, in message 5492d008026600086...@soto.provo.novell.com, Chun Yan Liu cy...@suse.com wrote: On 12/17/2014 at 06:52 PM, in message 20141217105227.gq136...@orkuz.home, Jiri Denemark jdene...@redhat.com wrote: On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote: Add public API virDomainSendSysrq for sending SysRequest key. Signed-off-by: Chunyan Liu cy...@suse.com --- changes: * add 'flags' to the new API * change parameter from 'const char *key' to 'char key' * change version number from 1.2.11 to 1.2.12 include/libvirt/libvirt-domain.h | 3 +++ src/driver-hypervisor.h | 4 src/libvirt-domain.c | 39 +++ src/libvirt_public.syms | 5 + 4 files changed, 51 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index baef32d..5f72850 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom, virDomainFSInfoPtr **info, unsigned int flags); +/* virDomainSendSysrq */ +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags); + I think quite a few reviewers (Daniel, Eric, and I) agreed on using an enum instead of char so that the API is more general. Sorry, I missed this part. I'll update. One left question: How about 'virsh sysrq' parameters? What would we expect users to pass? Any thoughts on that? libxl_send_sysrq Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what you had in mind for syntax previously - although it looks like: virsh sysrq domain [key] Thanks for reply. The syntax I'm used previously is: #virsh sysrq domain key key is required. It's just a letter, like 'h', 'c', etc. About which options can we have, on can refer to the results on guest through sysrq help. (that is, issue 'virsh sysrq domain h' and look at guest kernel message. I think on each guest, there must be 'h' option, it will print help message.) h, c, etc. doesn't tell me enough about what to expect from the perspective of this naive user... Passing 'h' via virsh to a driver to return some help string that gets displayed where? Guest kernel message if guest is Linux. xen/libxl just passes the key blindly to guest kernel, so to pass 'h' to guest kernel, it just like one issue: #echo 'h' /proc/sysrq-trigger in a Linux guest, you will see in /var/log/messages: SysRq : HELP : loglevel(0-9) reboot(b) crash(c) terminate-all-tasks(e) memory-full-oom-kill(f) kill-all-tasks(i) thaw-filesystems(j) sak(k) show-backtrace-all-active-cpus(l) show-memory-usage(m) nice-all-RT-tasks(n) poweroff(o) show-registers(p) show-all-timers(q) unraw(r) sync(s) show-task-states(t) unmount(u) force-fb(V) show-blocked-tasks(w) dump-ftrace-buffer(z) FWIW: My point on this was - by using 'virsh sysrq domain h' you don't provide a mechanism to display this output. Seems just from the descriptions some of those letters might return some useful information... Some aren't one way commands. Perhaps it would be useful to capture output and be able to return it... John Was there a mechanism I missed to return and display that output? Do you have sample output to show on a system with these changes applied? I don't know how if any other hypervisor behaves differently, for xen/libxl, they just send sysrq key to guest blindly if I understand correctly. So, which letter is corresponding to which option is all the same with guest sysrq key definition, in other words, it depends on guest sysrq key definition. Where if not provided key would be NULL, which doesn't look good for how the code reads now. As said above, key is required, it couldn't be NULL, otherwise, it will report error. While the check in virsh because VSH_OFLAG_REQ is set for key is good, what if someone calls the API directly? You have no check there for key being non null - it just gets passed along. The description for key in virDomainSendSysrq is still not sufficient to help me either: + * @key:SysRq key, like h, c, ... What does 'h', 'c', ... mean? What are the options? What do they map to functionality wise? I assume it's hypervisor dependent, but that's all stuff you need to describe somewhere. I don't
[libvirt] [PATCH] tests: Set up two more overrides for root builders
There are two more places after commit 3865941b that need to be adapted
in order to get rid of some test failures when building as root.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
tests/networkxml2firewalltest.c | 2 ++
tests/virfirewalltest.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c
index 1225c44..c69ab54 100644
--- a/tests/networkxml2firewalltest.c
+++ b/tests/networkxml2firewalltest.c
@@ -135,6 +135,8 @@ mymain(void)
ret = -1; \
} while (0)
+virFirewallSetLockOverride(true);
+
if (virFirewallSetBackend(VIR_FIREWALL_BACKEND_DIRECT) 0) {
ret = -1;
goto cleanup;
diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
index 81c5557..1f8d8f1 100644
--- a/tests/virfirewalltest.c
+++ b/tests/virfirewalltest.c
@@ -1168,6 +1168,8 @@ mymain(void)
RUN_TEST_DIRECT(name, method)
# endif /* ! WITH_DBUS */
+virFirewallSetLockOverride(true);
+
RUN_TEST(single group, testFirewallSingleGroup);
RUN_TEST(remove rule, testFirewallRemoveRule);
RUN_TEST(many groups, testFirewallManyGroups);
--
2.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 2/2] Add tests to xmconfigtest
Add tests to testing HVM default features (pae, acpi, apic)
conversion from xm config to libvirt xml. If no pae|acpi|apic
specified in xm config, after conversion, libvirt xml should
by default include:
features
pae/
apic/
acpi/
/features
Signed-off-by: Chunyan Liu cy...@suse.com
---
Changes to v1:
* add testcase to test HVM default features conversion from
xm config to xml.
* introduce DO_TEST_PARSE and DO_TEST_FORMAT to allow test
one direction only.
.../xmconfigdata/test-fullvirt-default-feature.cfg | 23 +++
.../xmconfigdata/test-fullvirt-default-feature.xml | 48 ++
tests/xmconfigtest.c | 20 -
3 files changed, 89 insertions(+), 2 deletions(-)
create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg
create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.xml
diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg
b/tests/xmconfigdata/test-fullvirt-default-feature.cfg
new file mode 100644
index 000..5ce234f
--- /dev/null
+++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg
@@ -0,0 +1,23 @@
+name = XenGuest2
+uuid = c7a5fdb2-cdaf-9455-926a-d65c16db1809
+maxmem = 579
+memory = 394
+vcpus = 1
+builder = hvm
+kernel = /usr/lib/xen/boot/hvmloader
+boot = d
+hpet = 1
+localtime = 0
+on_poweroff = destroy
+on_reboot = restart
+on_crash = restart
+device_model = /usr/lib/xen/bin/qemu-dm
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = 127.0.0.1
+vncpasswd = 123poi
+vif = [
mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu ]
+parallel = none
+serial = none
+disk = [ phy:/dev/HostVG/XenGuest2,hda,w, file:/root/boot.iso,hdc:cdrom,r ]
diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.xml
b/tests/xmconfigdata/test-fullvirt-default-feature.xml
new file mode 100644
index 000..57a6531
--- /dev/null
+++ b/tests/xmconfigdata/test-fullvirt-default-feature.xml
@@ -0,0 +1,48 @@
+domain type='xen'
+ nameXenGuest2/name
+ uuidc7a5fdb2-cdaf-9455-926a-d65c16db1809/uuid
+ memory unit='KiB'592896/memory
+ currentMemory unit='KiB'403456/currentMemory
+ vcpu placement='static'1/vcpu
+ os
+type arch='i686' machine='xenfv'hvm/type
+loader type='rom'/usr/lib/xen/boot/hvmloader/loader
+boot dev='cdrom'/
+ /os
+ features
+acpi/
+apic/
+pae/
+ /features
+ clock offset='utc' adjustment='reset'
+timer name='hpet' present='yes'/
+ /clock
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashrestart/on_crash
+ devices
+emulator/usr/lib/xen/bin/qemu-dm/emulator
+disk type='block' device='disk'
+ driver name='phy'/
+ source dev='/dev/HostVG/XenGuest2'/
+ target dev='hda' bus='ide'/
+/disk
+disk type='file' device='cdrom'
+ driver name='file'/
+ source file='/root/boot.iso'/
+ target dev='hdc' bus='ide'/
+ readonly/
+/disk
+interface type='bridge'
+ mac address='00:16:3e:66:92:9c'/
+ source bridge='xenbr1'/
+ script path='vif-bridge'/
+ model type='e1000'/
+/interface
+input type='mouse' bus='ps2'/
+input type='keyboard' bus='ps2'/
+graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'
passwd='123poi'
+ listen type='address' address='127.0.0.1'/
+/graphics
+ /devices
+/domain
diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c
index 0c6f803..8a49eb5 100644
--- a/tests/xmconfigtest.c
+++ b/tests/xmconfigtest.c
@@ -176,6 +176,7 @@ testCompareHelper(const void *data)
const struct testInfo *info = data;
char *xml = NULL;
char *cfg = NULL;
+char *cfgout = NULL;
if (virAsprintf(xml, %s/xmconfigdata/test-%s.xml,
abs_srcdir, info-name) 0 ||
@@ -191,6 +192,7 @@ testCompareHelper(const void *data)
cleanup:
VIR_FREE(xml);
VIR_FREE(cfg);
+VIR_FREE(cfgout);
return result;
}
@@ -207,18 +209,30 @@ mymain(void)
if (!(xmlopt = xenDomainXMLConfInit()))
return EXIT_FAILURE;
-#define DO_TEST(name, version) \
+#define DO_TEST_PARSE(name, version)\
do {\
struct testInfo info0 = { name, version, 0 }; \
-struct testInfo info1 = { name, version, 1 }; \
if (virtTestRun(Xen XM-2-XML Parse name,\
testCompareHelper, info0) 0) \
ret = -1; \
+} while (0)
+
+
+#define DO_TEST_FORMAT(name, version) \
+do {\
+struct testInfo info1 = { name, version, 1 }; \
if (virtTestRun(Xen XM-2-XML Format name,\
testCompareHelper,
[libvirt] [PATCH V2 0/2] fix xen HVM pae|apic|acpi features parser
According to xm.config manual, HVM pae|apic|acpi feature default is 1 (enabled). But in conversion from xm config to libvirt xml, if xm config doesn't contain pae|apic|acpi, it sets default value to 0, this causes some problems in HVM guest. Update parser codes to set HVM pae|apic|acpi default value to 1 to match xm config convension. Add tests data to test it. --- Changes to v1: * update xmconfigtest Chunyan Liu (2): xenconfig: set HVM pae/apic/acpi/ default to 1 Add tests to xmconfigtest src/xenconfig/xen_common.c | 6 +-- .../xmconfigdata/test-fullvirt-default-feature.cfg | 23 +++ .../xmconfigdata/test-fullvirt-default-feature.xml | 48 ++ tests/xmconfigtest.c | 20 - 4 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.xml -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 1/2] xenconfig: set HVM pae/apic/acpi/ default to 1
According to xm.config manual, HVM pae|apic|acpi feature default
is 1 (enabled). But in conversion from xm config to libvirt xml,
if xm config doesn't contain pae|apic|acpi, it sets default value
to 0, this causes some problems in HVM guest.
Update parser codes to set HVM pae|apic|acpi default value to 1
to match xm config convension.
Signed-off-by: Chunyan Liu cy...@suse.com
---
src/xenconfig/xen_common.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index 25bdf26..221509a 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -512,17 +512,17 @@ xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def)
return -1;
if (STREQ(def-os.type, hvm)) {
-if (xenConfigGetBool(conf, pae, val, 0) 0)
+if (xenConfigGetBool(conf, pae, val, 1) 0)
return -1;
else if (val)
def-features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON;
-if (xenConfigGetBool(conf, acpi, val, 0) 0)
+if (xenConfigGetBool(conf, acpi, val, 1) 0)
return -1;
else if (val)
def-features[VIR_DOMAIN_FEATURE_ACPI] = VIR_TRISTATE_SWITCH_ON;
-if (xenConfigGetBool(conf, apic, val, 0) 0)
+if (xenConfigGetBool(conf, apic, val, 1) 0)
return -1;
else if (val)
--
1.8.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3 1/5] Add public API virDomainSendSysrq
On 12/23/2014 at 11:42 AM, in message 5498e4ba.1000...@redhat.com, John Ferlan jfer...@redhat.com wrote: On 12/22/2014 09:55 PM, Chun Yan Liu wrote: On 12/22/2014 at 08:17 PM, in message 54980bf2.1060...@redhat.com, John Ferlan jfer...@redhat.com wrote: On 12/21/2014 10:15 PM, Chun Yan Liu wrote: On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, John Ferlan jfer...@redhat.com wrote: On 12/19/2014 12:31 AM, Chun Yan Liu wrote: On 12/18/2014 at 01:00 PM, in message 5492d008026600086...@soto.provo.novell.com, Chun Yan Liu cy...@suse.com wrote: On 12/17/2014 at 06:52 PM, in message 20141217105227.gq136...@orkuz.home, Jiri Denemark jdene...@redhat.com wrote: On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote: Add public API virDomainSendSysrq for sending SysRequest key. Signed-off-by: Chunyan Liu cy...@suse.com --- changes: * add 'flags' to the new API * change parameter from 'const char *key' to 'char key' * change version number from 1.2.11 to 1.2.12 include/libvirt/libvirt-domain.h | 3 +++ src/driver-hypervisor.h | 4 src/libvirt-domain.c | 39 +++ src/libvirt_public.syms | 5 + 4 files changed, 51 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index baef32d..5f72850 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom, virDomainFSInfoPtr **info, unsigned int flags); +/* virDomainSendSysrq */ +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags); + I think quite a few reviewers (Daniel, Eric, and I) agreed on using an enum instead of char so that the API is more general. Sorry, I missed this part. I'll update. One left question: How about 'virsh sysrq' parameters? What would we expect users to pass? Any thoughts on that? libxl_send_sysrq Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what you had in mind for syntax previously - although it looks like: virsh sysrq domain [key] Thanks for reply. The syntax I'm used previously is: #virsh sysrq domain key key is required. It's just a letter, like 'h', 'c', etc. About which options can we have, on can refer to the results on guest through sysrq help. (that is, issue 'virsh sysrq domain h' and look at guest kernel message. I think on each guest, there must be 'h' option, it will print help message.) h, c, etc. doesn't tell me enough about what to expect from the perspective of this naive user... Passing 'h' via virsh to a driver to return some help string that gets displayed where? Guest kernel message if guest is Linux. xen/libxl just passes the key blindly to guest kernel, so to pass 'h' to guest kernel, it just like one issue: #echo 'h' /proc/sysrq-trigger in a Linux guest, you will see in /var/log/messages: SysRq : HELP : loglevel(0-9) reboot(b) crash(c) terminate-all-tasks(e) memory-full-oom-kill(f) kill-all-tasks(i) thaw-filesystems(j) sak(k) show-backtrace-all-active-cpus(l) show-memory-usage(m) nice-all-RT-tasks(n) poweroff(o) show-registers(p) show-all-timers(q) unraw(r) sync(s) show-task-states(t) unmount(u) force-fb(V) show-blocked-tasks(w) dump-ftrace-buffer(z) FWIW: My point on this was - by using 'virsh sysrq domain h' you don't provide a mechanism to display this output. Seems just from the descriptions some of those letters might return some useful information... Some aren't one way commands. Perhaps it would be useful to capture output and be able to return it... Right. But might be hard. And I think of a problem when mapping enum to letter: to different guests (e.g. Linux vs Windows), the letter to enum mapping might be different, even hypervisor may not precisely know that. At least for xen hypervisor, I think it only blindly sends the key to guest, but has no idea of different key-letter meaning to different guests. - Chunyan John Was there a mechanism I missed to return and display that output? Do you have sample output to show on a system with these changes applied? I don't know how if any other hypervisor behaves differently, for xen/libxl, they just send sysrq key to guest blindly if I understand correctly. So, which letter is corresponding to which option is all the same with guest sysrq key definition, in other words, it depends on guest sysrq key definition.
