[libvirt] building fails with 0.8.6 and snapshot
hi. i am trying to build 0.8.6 but this fails due to the lack of macvtap enabled in my kernel. i won't use it, so i don't care, but even if i disable it, build won't work. apparently, thats a known bug, according to https://www.redhat.com/archives/libvir-list/2010-December/msg00034.html so now i am trying to build a snapshot but it also fails during "make rpm", specifically during make check in "tests". commandtest.c fails and I cannot find out why. here's the log: http://pastebin.com/rVPNDpS6 maybe someone can help me out? environment is opensuse 11.3 64 bit regards dominik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Update documentation of watchdog dump option and add test data for it
The xml watchdog dump option is converted to qemu watchdog pause arg
but it is not reasonable to convert it back from qemu watchdog pause
arg since there already is a xml watchdog pause option, so a test for
the dump option to convert it from arg to xml is not added.
---
I assume the dump option will appear in 0.8.7 and mark it since 0.8.7.
docs/formatdomain.html.in |8 +-
docs/schemas/domain.rng|1 +
.../qemuxml2argv-watchdog-dump.args|1 +
.../qemuxml2argv-watchdog-dump.xml | 26
tests/qemuxml2argvtest.c |1 +
5 files changed, 36 insertions(+), 1 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8db8b52..305bcb1 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1687,14 +1687,20 @@ qemu-kvm -net nic,model=? /dev/null
'poweroff' — forcefully power off the guest
'pause' — pause the guest
'none' — do nothing
+ 'dump' — automatic dump the guest
+Since 0.8.7
-Note that the 'shutdown' action requires that the guest
+Note 1: the 'shutdown' action requires that the guest
is responsive to ACPI signals. In the sort of situations
where the watchdog has expired, guests are usually unable
to respond to ACPI signals. Therefore using 'shutdown'
is not recommended.
+
+Note 2: the directory to save dump files can be configured
+by auto_dump_path in file /etc/libvirt/qemu.conf.
+
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 811d559..650ed7d 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -1472,6 +1472,7 @@
poweroff
pause
none
+dump
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.args
b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.args
new file mode 100644
index 000..50b26f8
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.args
@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M
pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait
-no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel
none -usb -watchdog ib700 -watchdog-action pause
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.xml
b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.xml
new file mode 100644
index 000..4314ec4
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.xml
@@ -0,0 +1,26 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219200
+ 219200
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 662f7bb..8328d65 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -399,6 +399,7 @@ mymain(int argc, char **argv)
DO_TEST("watchdog", 0, false);
DO_TEST("watchdog-device", QEMUD_CMD_FLAG_DEVICE |
QEMUD_CMD_FLAG_NODEFCONFIG, false);
+DO_TEST("watchdog-dump", 0, false);
DO_TEST("balloon-device", QEMUD_CMD_FLAG_DEVICE |
QEMUD_CMD_FLAG_NODEFCONFIG, false);
DO_TEST("balloon-device-auto", QEMUD_CMD_FLAG_DEVICE |
--
1.7.3
--
Thanks,
Hu Tao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RESEND] virsh: tell if running domain conf is updated
Based on "virDomainIsUpdated" API, to tell if the running domain
configuration is updated by "attach/detach/update device" and
"setvcpus" via "dominfo".
* tools/virsh.c
---
tools/virsh.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 4e37f2d..2960b79 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2038,6 +2038,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
virSecurityModel secmodel;
virSecurityLabel seclabel;
int persistent = 0;
+int updated = -1;
int ret = TRUE, autostart;
unsigned int id;
char *str, uuid[VIR_UUID_STRING_BUFLEN];
@@ -2099,6 +2100,15 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
else
vshPrint(ctl, "%-15s %s\n", _("Persistent:"), persistent ? _("yes") :
_("no"));
+/* Check and display whether the domain running configuration is
+ * updated or not.
+ */
+updated = virDomainIsUpdated(dom);
+if (updated < 0)
+vshPrint(ctl, "%-15s %s\n", _("Updated:"), _("unknown"));
+else
+vshPrint(ctl, "%-15s %s\n", _("Updated:"), updated ? _("yes") :
_("no"));
+
/* Check and display whether the domain autostarts or not */
if (!virDomainGetAutostart(dom, &autostart)) {
vshPrint(ctl, "%-15s %s\n", _("Autostart:"),
--
1.7.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [v2] qemu: Set domain def as updated and transient if changes
As qemu driver doesn't allow to make changes on persistent
domain configuration via "attach/detach/update device",
and all the changes made on the running domain configuration
should not be persistent across next boot (without the need
of restarting libvirtd), so:
1) Set the running domain def as transient, and restore
the domain configuration to original configuration when
shutdown.
2) Set the running domain def as updated, and reset it as
not updated when shutdown.
Also for "live VCPU set", it doesn't change the persistent
domain configuration, so, we also set the running domain
def as updated and transient, and restore the original def
when shutdown.
* src/qemu/qemu_driver.c
---
src/qemu/qemu_driver.c | 47 +++
1 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 19ce9a6..a3d87eb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4429,11 +4429,18 @@ retry:
VIR_FREE(priv->vcpupids);
priv->nvcpupids = 0;
+/* Restore original domain def, so that changes on running domain def
+ * will not be persistent across next boot.
+ */
if (vm->newDef) {
virDomainDefFree(vm->def);
vm->def = vm->newDef;
vm->def->id = -1;
vm->newDef = NULL;
+
+/* Now set domain def as not updated */
+if (vm->updated)
+vm->updated = 0;
}
if (orig_err) {
@@ -6473,7 +6480,17 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
break;
case VIR_DOMAIN_VCPU_LIVE:
+if (virDomainObjSetDefTransient(driver->caps, vm) < 0) {
+VIR_ERROR("Unable to set domain %s's running config as transient",
+vm->def->name);
+
+goto endjob;
+}
+
ret = qemudDomainHotplugVcpus(vm, nvcpus);
+
+if (ret == 0)
+vm->updated = 1;
break;
case VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG:
@@ -8819,6 +8836,13 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
goto endjob;
}
+if (virDomainObjSetDefTransient(driver->caps, vm) < 0) {
+VIR_ERROR("Unable to set domain %s's running config as transient",
+vm->def->name);
+
+goto endjob;
+}
+
dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
VIR_DOMAIN_XML_INACTIVE);
if (dev == NULL)
@@ -8916,6 +8940,9 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
goto endjob;
}
+if (ret == 0)
+vm->updated = 1;
+
if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
ret = -1;
@@ -9066,6 +9093,13 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
goto endjob;
}
+if (virDomainObjSetDefTransient(driver->caps, vm) < 0) {
+VIR_ERROR("Unable to set domain %s's running config as transient",
+vm->def->name);
+
+goto endjob;
+}
+
dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
VIR_DOMAIN_XML_INACTIVE);
if (dev == NULL)
@@ -9126,6 +9160,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
break;
}
+if (ret == 0)
+vm->updated = 1;
+
if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
ret = -1;
@@ -9786,6 +9823,13 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
goto endjob;
}
+if (virDomainObjSetDefTransient(driver->caps, vm) < 0) {
+VIR_ERROR("Unable to set domain %s's running config as transient",
+vm->def->name);
+
+goto endjob;
+}
+
dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
VIR_DOMAIN_XML_INACTIVE);
if (dev == NULL)
@@ -9828,6 +9872,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
"%s", _("This type of device cannot be hot
unplugged"));
}
+if (ret == 0)
+vm->updated = 1;
+
if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
ret = -1;
--
1.7.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 3/4] Add a watchdog action `dump'
On Fri, Dec 10, 2010 at 01:17:20AM +1100, Justin Clift wrote: > On 09/12/2010, at 9:55 PM, Daniel P. Berrange wrote: > > On Wed, Dec 08, 2010 at 02:19:17PM +0800, Hu Tao wrote: > >> `dump' watchdog action lets libvirtd to dump the guest when receives a > >> watchdog event (which probably means a guest crash) > >> > >> Currently only qemu is supported. > >> --- > >> src/conf/domain_conf.c |1 + > >> src/conf/domain_conf.h |1 + > >> src/qemu/libvirtd_qemu.aug |1 + > >> src/qemu/qemu.conf |5 ++ > >> src/qemu/qemu_conf.c| 16 ++- > >> src/qemu/qemu_conf.h|5 ++ > >> src/qemu/qemu_driver.c | 96 > >> +++ > >> src/qemu/test_libvirtd_qemu.aug |2 + > >> 8 files changed, 126 insertions(+), 1 deletions(-) > > Doesn't seem to be any updates to the documentation with the patch, > explaining what it does > and how to use it? Thank you for pointing it out! -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/8] Perform a handshake during QEMU startup to sync with relabelling
On 11/22/2010 11:09 AM, Daniel P. Berrange wrote:
> Perform a handshake during QEMU startup to allow file relabelling
> to be performed after QEMU has been forked, but before it has
> been exec'd. This is to allow the lock manager to acquire locks
> against the QEMU pid, before relabelling takes place
>
> * src/qemu/qemu_driver.c: Handshake during QEMU startup
> ---
> src/qemu/qemu_driver.c | 23 ++-
> 1 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0b0b6fe..bf6cd0e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3683,9 +3683,6 @@ static int qemudSecurityHook(void *data) {
> if (virSecurityManagerSetProcessLabel(h->driver->securityManager, h->vm)
> < 0)
> goto cleanup;
>
> -/* XXX temp hack to let disk labelling complete */
> -sleep(10);
> -
> ret = 0;
>
> cleanup:
> @@ -4097,28 +4094,36 @@ static int qemudStartVMDaemon(virConnectPtr conn,
> virCommandSetErrorFD(cmd, &logfile);
> virCommandSetPidFile(cmd, pidfile);
> virCommandDaemonize(cmd);
> +virCommandRequireHandshake(cmd);
>
> ret = virCommandRun(cmd, NULL);
Oh, I see - you want to daemonize, require handshake, AND do a blocking
virCommandRun. I see now - __virExec does two forks when daemonize is
requested, and only runs the callback hook in the second (daemon) child,
rather than in the (short-lived) intermediary. That answers some of my
questions in the previous patch.
> -VIR_WARN("Executing done %s", vm->def->emulator);
> VIR_FREE(pidfile);
>
> -/* XXX this is bollocks. Need a sync point */
> -sleep(5);
> +VIR_WARN0("Waiting for handshake from child");
> +if (virCommandHandshakeWait(cmd) < 0) {
But it still probably requires virCommand to do some sanity checking
that HandshakeWait is called after the child has been spawned.
>
> +VIR_WARN0("Labelling done, completing hanshake to child");
s/hanshake/handshake/
Definitely an improvement.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 v6 3/4] Add a watchdog action `dump'
On Thu, Dec 09, 2010 at 12:00:48PM -0700, Eric Blake wrote: > On 12/07/2010 11:19 PM, Hu Tao wrote: > > `dump' watchdog action lets libvirtd to dump the guest when receives a > > watchdog event (which probably means a guest crash) > > > > Currently only qemu is supported. > > --- > > src/conf/domain_conf.c |1 + > > src/conf/domain_conf.h |1 + > > src/qemu/libvirtd_qemu.aug |1 + > > src/qemu/qemu.conf |5 ++ > > src/qemu/qemu_conf.c| 16 ++- > > src/qemu/qemu_conf.h|5 ++ > > src/qemu/qemu_driver.c | 96 > > +++ > > src/qemu/test_libvirtd_qemu.aug |2 + > > 8 files changed, 126 insertions(+), 1 deletions(-) > > Needed a bit of conflict resolution given other patches that have gone > in since this was posted, as well as a tweak to pass 'make check' > (test_libvirtd_qemu.aug was incomplete), but I've now pushed this patch > since it had an ACK. As Daniel said, I'm omitting patch 4, and looking > forward to his larger rewrite (which I did promise to review: > https://www.redhat.com/archives/libvir-list/2010-December/msg00073.html). > > However, as Justin pointed out, the new dump option still needs > documentation and testing: > - mention 'dump' in docs/formatdomain.html.in > - update to docs/schemas/domain.rng to list dump as a valid option > - add an .xml file somewhere under tests/ that exercises the new XML option OK, will update it. -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 1/4] threadpool impl
On Thu, Dec 09, 2010 at 10:53:56AM +, Daniel P. Berrange wrote: > On Wed, Dec 08, 2010 at 02:19:06PM +0800, Hu Tao wrote: > > * src/util/threadpool.c, src/util/threadpool.h: Thread pool > > implementation > > * src/Makefile.am: Build thread pool > > * src/libvirt_private.syms: Export public functions > > --- > > cfg.mk |1 + > > src/Makefile.am |1 + > > src/libvirt_private.syms |6 + > > src/util/threadpool.c| 231 > > ++ > > src/util/threadpool.h| 48 ++ > > 5 files changed, 287 insertions(+), 0 deletions(-) > > create mode 100644 src/util/threadpool.c > > create mode 100644 src/util/threadpool.h > > ACK Thanks:) -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 1/4] threadpool impl
On Thu, Dec 09, 2010 at 11:40:32AM -0700, Eric Blake wrote: > On 12/07/2010 11:19 PM, Hu Tao wrote: > > I fixed your 'make syntax-check' problems: Thanks! -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] Allow handshake with child process during startup
On 11/22/2010 11:09 AM, Daniel P. Berrange wrote:
[reviving this thread a bit]
> Allow the parent process to perform a bi-directional handshake
> with the child process during fork/exec. The child process
> will fork and do its initial setup. Immediately prior to the
> exec(), it will stop & wait for a handshake from the parent
> process. The parent process will spawn the child and wait
> until the child reaches the handshake point. It will do
> whatever extra setup work is required, before signalling the
> child to continue.
>
> The implementation of this is done using two pairs of blocking
> pipes. The first pair is used to block the parent, until the
> child writes a single byte. Then the second pair pair is used
> to block the child, until the parent confirms with another
> single byte.
>
> * src/util/command.c, src/util/command.h,
> src/libvirt_private.syms: Add APIs to perform a handshake
> ---
> src/libvirt_private.syms |3 +
> src/util/command.c | 92
> --
> src/util/command.h |5 ++
> 3 files changed, 96 insertions(+), 4 deletions(-)
>
> @@ -626,8 +632,8 @@ int virCommandRun(virCommandPtr cmd,
> ret = -1;
>
> VIR_DEBUG("Result stdout: '%s' stderr: '%s'",
> - NULLSTR(*cmd->outbuf),
> - NULLSTR(*cmd->errbuf));
> + cmd->outbuf ? NULLSTR(*cmd->outbuf) : "(null)",
> + cmd->errbuf ? NULLSTR(*cmd->errbuf) : "(null)");
Ah - I see you stumbled on the same problem that has already been fixed.
You won't need this hunk :)
>
> +static int virCommandHookImpl(void *data)
> +{
Hmm, I already implemented a static function virCommandHook() for
managing cwd swapping; this should be merged into that function.
> +virCommandPtr cmd = data;
> +
> +if (cmd->hook &&
> +cmd->hook(cmd->opaque) < 0)
> +return -1;
> +
> +if (cmd->handshake) {
> +char c = 'w';
> +VIR_WARN0("Notifying parent for handshake start");
> +if (safewrite(cmd->handshakeWait[1], &c, sizeof(c)) != sizeof(c)) {
> +virReportSystemError(errno, "%s", _("Unable to notify parent
> process"));
> +return -1;
> +}
> +VIR_WARN0("Waiting on parent for handshake complete ");
trailing space
> +if (saferead(cmd->handshakeNotify[0], &c, sizeof(c)) != sizeof(c)) {
> +virReportSystemError(errno, "%s", _("Unable to wait on parent
> process"));
> +return -1;
> +}
> +if (c != 'n') {
> +virReportSystemError(EINVAL, _("Unexpected data '%d' from parent
> process"), (int)c);
> +return -1;
> +}
> +VIR_WARN0("Handshake is done, child is running");
> +VIR_FORCE_CLOSE(cmd->handshakeWait[1]);
> +VIR_FORCE_CLOSE(cmd->handshakeNotify[0]);
> +}
> +
> +return 0;
> +}
Looks correct for the hook.
>
>
> +void virCommandRequireHandshake(virCommandPtr cmd)
> +{
if (!cmd || cmd->has_error) return;
> +if (pipe(cmd->handshakeWait) < 0)
> +cmd->has_error = errno;
> +if (pipe(cmd->handshakeNotify) < 0) {
> +VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
> +VIR_FORCE_CLOSE(cmd->handshakeWait[1]);
> +}
Oops; forgot to set cmd->has_error in this case. And this is the first
instance of setting cmd->has_error to an errno other than ENOMEM; we'll
have to make sure virCommandRun() does the right thing in this case.
Also, we should have a sanity check that use of RequireHandshake entails
that virCommandRunAsync (and not virCommandRun) is called, so that we
don't deadlock. Can RequireHandshake and Daemonize be mixed, or should
our sanity checks declare them as incompatible?
> +
> +virCommandPreserveFD(cmd, cmd->handshakeWait[1]);
> +virCommandPreserveFD(cmd, cmd->handshakeNotify[0]);
Would these be better as virCommandTransferFD()?
> +cmd->handshake = true;
> +}
> +
> +int virCommandHandshakeWait(virCommandPtr cmd)
> +{
> +char c;
Needs error checking that the command has been started but not yet
waited on (that is, cmd->pid should be non-negative).
> +if (saferead(cmd->handshakeWait[0], &c, sizeof(c)) != sizeof(c)) {
> +virReportSystemError(errno, "%s", _("Unable to wait for child
> process"));
> +return -1;
> +}
> +if (c != 'w') {
> +virReportSystemError(EINVAL, _("Unexpected data '%d' from child
> process"), (int)c);
> +return -1;
> +}
> +return 0;
> +}
> +
> +int virCommandHandshakeNotify(virCommandPtr cmd)
> +{
> +char c = 'n';
Likewise. Should this also check that handshakewait has been called first?
> +if (safewrite(cmd->handshakeNotify[1], &c, sizeof(c)) != sizeof(c)) {
> +virReportSystemError(errno, "%s", _("Unable to notify child
> process"));
> +return -1;
> +}
> +return 0;
> +}
> +
> +
> +void virCommandRequireHandshake(virCommandPtr cmd);
> +
> +int virCommandHandshakeWait(virCommandPtr cmd
Re: [libvirt] [PATCH] spec: do not start libvirt-guests if that service is off
On 12/07/2010 09:05 AM, Dan Kenigsberg wrote: > starting a service during rpm installation is impolite. It is even worse if > done > during upgrade, for a service that was explicitly turned off. > --- > libvirt.spec.in |8 +--- > 1 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/libvirt.spec.in b/libvirt.spec.in > index 48453d7..e0cab78 100644 > --- a/libvirt.spec.in > +++ b/libvirt.spec.in > @@ -700,9 +700,11 @@ fi > /sbin/ldconfig > /sbin/chkconfig --add libvirt-guests > if [ $1 -ge 1 ]; then > -# this doesn't do anything but allowing for libvirt-guests to be > -# stopped on the first shutdown > -/sbin/service libvirt-guests start > /dev/null 2>&1 || true > +if /sbin/chkconfig --list libvirt-guests | /bin/grep -q :on ; then grep -q is not POSIX, but then again, in a spec file, where you can assume chkconfig exists, you can assume grep -q exists. I was going to suggest saving a process: case $(/sbin/chkconfig --list libvirt-guests) in *:on*) ... esac but it's not worth it, for how seldom the shell code in a spec file is run. > +# this doesn't do anything but allowing for libvirt-guests to be > +# stopped on the first shutdown > +/sbin/service libvirt-guests start > /dev/null 2>&1 || true > +fi > fi ACK, and pushed as-is. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] esx: Refactor storage pool type lookup into a function
On 12/06/2010 01:10 PM, Matthias Bolte wrote: > --- > src/esx/esx_storage_driver.c | 130 > ++ > 1 files changed, 56 insertions(+), 74 deletions(-) > > diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c > index 83b53fb..9165f7a 100644 > --- a/src/esx/esx_storage_driver.c > +++ b/src/esx/esx_storage_driver.c > @@ -49,6 +49,58 @@ verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN); ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] esx: Improve error reporting for failed tasks
On 12/06/2010 06:45 AM, Matthias Bolte wrote: > Instead of just reporting that a task failed get the > localized message from the TaskInfo error and include > it in the reported error message. > > Implement minimal deserialization support for the > MethodFault type in order to obtain the actual fault > type. > > For example, this changes the reported error message > when trying to create a volume with zero size from > > Could not create volume > > to > > Could not create volume: InvalidArgument - A specified parameter was not > correct. > > Not perfect yet, but better than before. Certainly an improvement. ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] Missing "Default-Stop" field in LSB comment in libvirt-guests
On 12/05/2010 11:49 AM, Laurent Léonard wrote: > Hi, > > The "Default-Stop" field in LSB comment in libvirt-guests is missing and > should > be added. I also suggests to add runlevel 2 to the "Default-Start" field. > > The attached patch fixes that issue. Can you point to a URL that describes the Default-Stop field? The patch seems simple enough, but I want to know what various distros use for runlevel 2, or see it spelled out in the LSB, before I know for sure whether this is safe to ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 8/8] Start of a lock manager daemon
On 12/01/2010 10:26 AM, Daniel P. Berrange wrote:
> This starts the basic framework for a lock manager daemon to
> provide guarenteed isolation between VMs using disk images.
> It is a simple demo of how the generic RPC server APIs are
> to be used
> ---
> src/Makefile.am | 16 ++
> src/virtlockd.c | 620
> +++
> 2 files changed, 636 insertions(+), 0 deletions(-)
> create mode 100644 src/virtlockd.c
Not really part of the XDR refactoring series.
> +
> +
> +static int daemonForkIntoBackground(const char *argv0)
> +{
> +int statuspipe[2];
> +if (pipe(statuspipe) < 0)
> +return -1;
Does this overlap with virCommand?
I didn't really review this one too closely.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 7/8] Convert the remote driver to new RPC client APIs
On 12/01/2010 10:26 AM, Daniel P. Berrange wrote:
> This guts the current remote driver, removing all its networking
> handling code. Instead it calls out to the new virClientPtr and
> virClientProgramPtr APIs for all RPC & networking work.
> ---
> src/Makefile.am|3 +-
> src/remote/remote_driver.c | 2530
>
> 2 files changed, 419 insertions(+), 2114 deletions(-)
And here we start to see the real payoff of this series.
>
> -if(VIR_ALLOC(priv->callbackList)<0) {
> +if (VIR_ALLOC(priv->callbackList)<0) {
> virReportOOMError();
> goto failed;
> }
>
> -if(VIR_ALLOC(priv->domainEvents)<0) {
> +if (VIR_ALLOC(priv->domainEvents)<0) {
Can you separate some of this formatting touchups into an independent patch?
> @@ -8165,7 +7442,7 @@ done:
> return rv;
> }
>
> -
> +#if 0
> static struct private_stream_data *
> remoteStreamOpen(virStreamPtr st,
> int output ATTRIBUTE_UNUSED,
> @@ -8712,7 +7989,7 @@ done:
I see - another case where stream support is missing.
But looking like a nice start.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 6/8] Introduce generic RPC client objects
On 12/01/2010 10:26 AM, Daniel P. Berrange wrote:
> To facilitate creation of new clients using XDR RPC services,
> pull alot of the remote driver code into a set of reusable
s/alot/a lot/
> objects.
>
> - virNetClient: Encapsulates a socket connection to a
>remote RPC server. Handles all the network I/O for
>reading/writing RPC messages. Delegates RPC encoding
>and decoding to the registered programs
>
> - virNetClientProgram: Handles processing and dispatch
>of RPC messages for a single RPC (program,version).
>A program can register to receive async events
>from a client
> - virNetClientSASLContext: Handles everything todo with
>SASL authentication and encryption. The callers no
>longer need directly call any cyrus-sasl APIs, which
>means error handling is simpler & alternative SASL
>impls can be provided for Win32
>
> Each new client program now merely needs to define the list of
> RPC procedures & events it wants and their handlers. It does
> not need to deal with any of the network I/O functionality at
> all.
> +++ b/src/Makefile.am
> @@ -1117,7 +1117,7 @@ libvirt_qemu_la_LIBADD = libvirt.la
> $(CYGWIN_EXTRA_LIBADD)
> EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
>
>
> -noinst_LTLIBRARIES += libvirt-net-rpc.la libvirt-net-rpc-server.la
> +noinst_LTLIBRARIES += libvirt-net-rpc.la libvirt-net-rpc-server.la
> libvirt-net-rpc-client.la
Wrap at 80 columns.
>
> libvirt_net_rpc_la_SOURCES = \
> ../daemon/event.c \
> @@ -1153,6 +1153,18 @@ libvirt_net_server_la_LDFLAGS = \
> libvirt_net_server_la_LIBADD = \
> $(CYGWIN_EXTRA_LIBADD)
>
> +libvirt_net_client_la_SOURCES = \
> + rpc/virnetclientsaslcontext.h rpc/virnetclientsaslcontext.c \
> + rpc/virnetclientprogram.h rpc/virnetclientprogram.c \
> + rpc/virnetclient.h rpc/virnetclient.c
> +libvirt_net_client_la_CFLAGS = \
> + $(AM_CFLAGS)
> +libvirt_net_client_la_LDFLAGS = \
> + $(AM_LDFLAGS) \
> + $(CYGWIN_EXTRA_LDFLAGS) \
> + $(MINGW_EXTRA_LDFLAGS)l
s/l$//
> +++ b/src/rpc/virnetclient.c
> @@ -0,0 +1,1237 @@
> +
> +
Copyright header? Probably affects multiple new files across multiple
of these patches.
> +
> +#ifdef WIN32
> +# define pipe(fds) _pipe(fds,4096, _O_BINARY)
> +#endif
Yuck. Gnulib should really take care of this for us. But for now, we
have to keep it.
> +
> +virNetClientPtr virNetClientNewCommand(const char **cmdargv,
> + const char **cmdenv)
> +{
If virNetSocketNewConnectCommand is rewritten around virCommand, then
this should be updated as well.
> +
> +static int virNetClientCallDispatchStream(virNetClientPtr client
> ATTRIBUTE_UNUSED)
> +{
> +#if 0
> +struct private_stream_data *privst;
You weren't kidding about this being an incomplete RFC series.
> +#define __VIR_NET_CLIENT_H__
> +
> +#if HAVE_SASL
'make syntax-check' won't like this if you have cppi installed.
Again, mostly okay; just copying existing code and renaming into new
API. Overall, I'm liking the direction this series is heading.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 5/8] Introduce generic RPC server objects
On 12/01/2010 10:26 AM, Daniel P. Berrange wrote:
> To facilitate creation of new daemons providing XDR RPC services,
> pull alot of the libvirtd daemon code into a set of reusable
> objects.
>
> * virNetServer: A server contains one or more services which
>accept incoming clients. It maintains the list of active
>clients. It has a list of RPC programs which can be used
>by clients. When clients produce a complete RPC message,
>the server passes this onto the corresponding program for
>handling, and queues any response back with the client.
>
> * virNetServerClient: Encapsulates a single client connection.
>All I/O for the client is handled, reading & writing RPC
>messages. Also contains the SASL/TLS code, but this will
>eventually move into the virNetSocket object
>
> * virNetServerProgram: Handles processing and dispatch of
>RPC method calls for a single RPC (program,version).
>Multiple programs can be registered with the server.
>
> * virNetServerService: Encapsulates socket(s) listening for
>new connections. Each service listens on a single host/port,
>but may have multiple sockets if on a dual IPv4/6 host.
>
> Each new daemon now merely has to define the list of RPC procedures
> & their handlers. It does not need to deal with any network related
> functionality at all.
> @@ -1138,6 +1138,22 @@ libvirt_net_rpc_la_LDFLAGS = \
> libvirt_net_rpc_la_LIBADD = \
> $(CYGWIN_EXTRA_LIBADD)
>
> +libvirt_net_server_la_SOURCES = \
> + rpc/virnetservermessage.h \
> + rpc/virnetserverprogram.h rpc/virnetserverprogram.c \
> + rpc/virnetserverservice.h rpc/virnetserverservice.c \
> + rpc/virnetserverclient.h rpc/virnetserverclient.c \
> + rpc/virnetserver.h rpc/virnetserver.c
> +libvirt_net_server_la_CFLAGS = \
> + $(AM_CFLAGS)
> +libvirt_net_server_la_LDFLAGS = \
> + $(AM_LDFLAGS) \
> + $(CYGWIN_EXTRA_LDFLAGS) \
> + $(MINGW_EXTRA_LDFLAGS)l
s/l$//
> +
> +struct _virNetServer {
> +int refs;
size_t?
> +static void virNetServerHandleJob(void *jobOpaque, void *opaque)
> +{
> +virNetServerPtr srv = opaque;
> +virNetServerJobPtr job = jobOpaque;
> +virNetServerProgramPtr prog = NULL;
> +int i;
s/int/size_t/
> +
> +if (virNetServerProgramDispatch(prog,
> +srv,
> +job->client,
> +job->msg) < 0) {
> +job->msg = NULL;
> +goto error;
> +}
Should this call be run while the virNetServerLock is still held, or
should we drop and reacquire the lock to allow other virNetServer
threads to make progress during the arbitrarily long dispatch?
> +static int virNetServerDispatchNewClient(virNetServerServicePtr svc
> ATTRIBUTE_UNUSED,
> + virNetServerClientPtr client,
> + void *opaque)
> +{
> +virNetServerPtr srv = opaque;
> +
> +virNetServerLock(srv);
> +
> +if (srv->nclients >= srv->nclients_max) {
> +virNetError(VIR_ERR_RPC,
> +_("Too many active clients (%d), dropping connection
> from %s"),
> +(int)srv->nclients_max,
> virNetServerClientAddrString(client));
You can safely use %zu to avoid the case of size_t srv->nclients_max.
> +
> +#if 0
> +reprocess:
> +for (i = 0 ; i < srv->nclients ; i++) {
> +int inactive;
Any reason to keep this block of code, since you commented it out?
> +static void
> +virNetServerProgramFormatError(virNetServerProgramPtr prog,
> + void *rerr,
> + int code,
> + const char *fmt,
> + ...)
> +{
> +va_list args;
> +char msgbuf[1024];
> +char *msg = msgbuf;
> +
> +va_start(args, fmt);
> +vsnprintf(msgbuf, sizeof msgbuf, fmt, args);
Should we do something special if the message got truncated at
sizeof(msgbuf)?
> +
> +virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
> + const char *service,
> + int auth,
> + virNetTLSContextPtr tls)
> +{
> +virNetServerServicePtr svc;
> +int i;
s/int/size_t/
Mostly looks reasonable (probably because it's mostly copying from
existing working code, and just renaming into the new API names).
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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 4/8] Introduce generic objects for building XDR RPC servers/clients
On 12/01/2010 10:26 AM, Daniel P. Berrange wrote:
> Introduces a set of generic objects which are to be used in
> building RPC servers/clients based on XDR.
>
> - virNetMessageHeader - standardize the XDR format for any
>RPC program. Copied from remote protocol for back compat
>
> - virNetMessage - Provides a buffer for (de-)serializing
>messages, and a copy of the decoded virNetMessageHeader.
>Provides APIs for encoding/decoding message headers and
>payloads, thus isolating all the XDR api calls in one
>file. Callers no longer need to use XDR themselves.
>
> - virNetSocket - a wrapper around a socket file descriptor,
>to simplify creation of new sockets, both for clients and
>services. Encapsulates all the hairy getaddrinfo code
>and sockaddr manipulation. Will eventually include
>transparent support for TLS and SASL encoding of data
>
> - virNetTLSContext - encapsulates the credentials required
>to setup TLS sessions. eg the set of x509 certificates
>and keys, optional DH parameters and x509 DName whitelist
>Provides APIs for easily validating certificates from a
>TLS session
>
> - virNetTLSSession - encapsulates the TLS session handling,
>so that callers no longer have a direct dependancy on
>gnutls. This will facilitate adding alternate TLS impls.
>Makes the read/write TLS functions work with same
>semantics as the native socket read/write functions. ie
>they set errno, instead of a gnutls specific error code.
>
> This code is taken from either the daemon/libvirtd.c,
> daemon/dispatch.c or src/remote/remote_driver.c files,
> which all duplicated alot of functionality.
Whether or not you decide to break this into multiple patches (one per
API addition) or keep it as one (since this is all pretty much pure
addition), here's my first round of review:
> @@ -1098,6 +1116,28 @@ libvirt_qemu_la_CFLAGS = $(AM_CFLAGS)
> libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD)
> EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
>
> +
> +noinst_LTLIBRARIES += libvirt-net-rpc.la
> +
> +libvirt_net_rpc_la_SOURCES = \
> + ../daemon/event.c \
> + rpc/virnetprotocol.h rpc/virnetprotocol.c \
> + rpc/virnetmessage.h rpc/virnetmessage.c \
> + rpc/virnettlscontext.h rpc/virnettlscontext.c \
> + rpc/virnetsocket.h rpc/virnetsocket.c
> +libvirt_net_rpc_la_CFLAGS = \
> + $(GNUTLS_CFLAGS) \
> + $(SASL_CFLAGS) \
> + $(AM_CFLAGS)
If my cygwin patch is approved first, this will need $(XDR_CFLAGS).
https://www.redhat.com/archives/libvir-list/2010-December/msg00404.html
> +++ b/src/rpc/virnetmessage.c
> @@ -0,0 +1,215 @@
> +#include
> +
> +#include "virnetmessage.h"
> +
> +#include "virterror_internal.h"
> +#include "logging.h"
> +
> +#define virNetError(code, ...)\
> +virReportErrorHelper(NULL, VIR_FROM_RPC, code, __FILE__, \
> + __FUNCTION__, __LINE__, __VA_ARGS__)
> +
Does this need #define VIR_FROM_THIS VIR_FROM_RPC?
Should virNetError be added to the list of error() functions in cfg.mk?
> +++ b/src/rpc/virnetmessage.h
> @@ -0,0 +1,31 @@
> +#ifndef __VIR_NET_MESSAGE_H__
> +#define __VIR_NET_MESSAGE_H__
> +
> +#include "virnetprotocol.h"
> +
> +typedef struct virNetMessageHeader *virNetMessageHeaderPtr;
> +
> +typedef struct _virNetMessage virNetMessage;
> +typedef virNetMessage *virNetMessagePtr;
> +
> +struct _virNetMessage {
> +char buffer[VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX];
> +unsigned int bufferLength;
> +unsigned int bufferOffset;
> +
> +virNetMessageHeader header;
> +};
That's a big struct; where lots of space will typically be unused. It
should never be stack-allocated. Should we rearrange the fields to put
buffer last, and then use variable-sized array (or something similar) to
trim the size down to what is needed, rather than always allocating the
largest possible message?
> +
> +static int virNetSocketForkDaemon(const char *binary)
> +{
> +const char *const daemonargs[] = { binary, "--timeout=30", NULL };
> +pid_t pid;
> +
> +if (virExecDaemonize(daemonargs, NULL, NULL,
> + &pid, -1, NULL, NULL,
> + VIR_EXEC_CLEAR_CAPS,
> + NULL, NULL, NULL) < 0)
> +return -1;
Should this use virCommand instead?
> +
> +#if 0
> +/* There is no meaningful local addr for UNIX sockets,
> + * and getsockname() returns something with AF_INET
> + * in sa_family when run against AF_JUNIX sockets !
Is it worth copying this typo around?
> +
> +int virNetSocketNewConnectSSH(const char *nodename,
> + const char *service,
> + const char *binary,
> + const char *username,
> + bool noTTY,
> + const char *netcat,
> +
Re: [libvirt] [PATCH 2/2] tests: Add tests for network disks
On 12/09/2010 04:10 AM, Daniel P. Berrange wrote: > On Tue, Dec 07, 2010 at 11:57:33AM -0800, Josh Durgin wrote: >> >> Signed-off-by: Josh Durgin >> --- >> tests/qemuargv2xmltest.c |3 ++ >> .../qemuxml2argv-disk-drive-network-nbd.args |1 + >> .../qemuxml2argv-disk-drive-network-nbd.xml| 32 ++ >> .../qemuxml2argv-disk-drive-network-rbd.args |1 + >> .../qemuxml2argv-disk-drive-network-rbd.xml| 34 >> >> .../qemuxml2argv-disk-drive-network-sheepdog.args |1 + >> .../qemuxml2argv-disk-drive-network-sheepdog.xml | 32 ++ >> tests/qemuxml2argvtest.c |6 +++ >> 8 files changed, 110 insertions(+), 0 deletions(-) >> create mode 100644 >> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args >> create mode 100644 >> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml >> create mode 100644 >> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args >> create mode 100644 >> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml >> create mode 100644 >> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args >> create mode 100644 >> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml > > ACK Both patches are now pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] add network disk support
On 12/09/2010 12:13 PM, Eric Blake wrote: > On 12/09/2010 04:09 AM, Daniel P. Berrange wrote: >> On Mon, Dec 06, 2010 at 04:24:09PM +0900, MORITA Kazutaka wrote: >>> This patch adds network disk support to libvirt/QEMU. The currently >>> supported protocols are nbd, rbd, and sheepdog. The XML syntax is like >>> this: >>> > >> ACK > > NACK. After applying this patch, I get 'make check' failures: > > TEST: vmx2xmltest > /bin/sh: line 5: 15285 Segmentation fault (core dumped) Weird. After 'make clean', I was no longer able to reproduce the failure; I then did a complete fresh checkout to verify on a pristine tree. I must have had some bad cruft lying around in my tree, and wonder what it was about your patch to expose that cruft? At any rate, given that your patch does not fail after all, I will go ahead and push it. However, we still need a follow-up patch: > Hmm; missing a corresponding update to docs/formatdomain.html.in to > document the new XML. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] [PATCHv2] PHYP: Adding network interface management
On 11/22/2010 06:03 PM, Eduardo Otubo wrote:
Apologies for my review backlog (if you haven't guessed, I sometimes
table big patches for later, then forget to come back rapidly).
> This is the implementation of the previous patch now using virInterface*
> API. Ended up this patch got much more simpler, smaller and easier to
> review. Here is some details:
>
> * MAC size and interface name are fixed due to specifications on HMC,
>both are created automatically and CAN'T be specified from user. They
>have the following format:
>
> * MAC: 122980003002
HMC represents MAC as a decimal number? How hideous, but not your
fault; and this comment in the commit message will be useful for anyone
auditing the code in the future.
> +/* Getting the remote slot number */
> +
> +char_ptr = NULL;
> +char_ptr = strchr(iface->mac, '\n');
Redundant assignments (delete the first line).
> +if (VIR_ALLOC_N(name, PHYP_IFACENAME_SIZE) < 0) {
> +virReportOOMError();
> +goto err;
> +}
> +
> +if (VIR_ALLOC_N(mac, PHYP_MAC_SIZE) < 0) {
> +virReportOOMError();
> +goto err;
> +}
Since these arrays are so small, it's faster to just stack-allocate them:
char name[PHYP_IFACENAME_SIZE];
> +/* The next free slot itself: */
> +slot++;
> +
> +/* Now addig the new network interface */
s/addig/adding/
> +if (exit_status < 0 || ret == NULL)
> +goto err;
> +
> +char_ptr = NULL;
> +char_ptr = strchr(ret, '\n');
Another redundant assignment.
> +if (memcpy(mac, ret, PHYP_IFACENAME_SIZE) == NULL)
> +goto err;
> +
> +VIR_FREE(cmd);
> +VIR_FREE(ret);
> +return virGetInterface(conn, name, mac);
> +
> + err:
> +VIR_FREE(name);
> +VIR_FREE(cmd);
> +VIR_FREE(ret);
> +return NULL;
This leaks name if you end up calling virGetInterface. And both paths
leak mac. But if you change name and mac to be stack-allocated, then
you don't have to worry about freeing them.
> +ret = phypExec(session, cmd, &exit_status, conn);
> +
> +if (exit_status < 0 || ret == NULL)
> +goto err;
> +
> +VIR_FREE(cmd);
> +return virGetInterface(conn, name, ret);
> +
> + err:
> +VIR_FREE(cmd);
> +VIR_FREE(ret);
> +return NULL;
This leaks ret if virGetInterface is called.
> +ret = phypExec(session, cmd, &exit_status, iface->conn);
> +
> +if (exit_status < 0 || ret == NULL)
> +goto err;
> +
> +if (virStrToLong_i(ret, &char_ptr, 10, &state) == -1)
> +goto err;
> +
> +if (char_ptr)
> +*char_ptr = '\0';
> +
> +VIR_FREE(cmd);
> +return state;
Another leak of ret.
> +static int
> +phypListInterfaces(virConnectPtr conn, char **const names, int nnames)
s/int/size_t/
> +ret = phypExec(session, cmd, &exit_status, conn);
> +
> +/* I need to parse the textual return in order to get the network
> interfaces */
> +if (exit_status < 0 || ret == NULL)
> +goto err;
> +else {
Rather than indenting the rest of the function inside an else{} block,
you can leave the code at the top level indentation since the if() block
was an unconditional goto.
> +ret = phypExec(session, cmd, &exit_status, conn);
> +
> +if (exit_status < 0 || ret == NULL)
> +goto err;
> +
> +if (virStrToLong_i(ret, &char_ptr, 10, &nnets) == -1)
> +goto err;
> +
> +if (char_ptr)
> +*char_ptr = '\0';
> +
> +VIR_FREE(cmd);
> +return nnets;
Another leak of ret.
>
> int
> @@ -4117,7 +4651,7 @@ phypRegister(void)
> return -1;
> if (virRegisterStorageDriver(&phypStorageDriver) < 0)
> return -1;
> -if (virRegisterNetworkDriver(&phypNetworkDriver) < 0)
> +if (virRegisterInterfaceDriver(&phypInterfaceDriver) < 0)
Are you intending to replace NetworkDriver with InterfaceDriver, or
should you be supporting both drivers simultaneously (although this may
be more an indication of how unfamiliar I am with the difference between
what the two drivers are supposed to provide).
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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] add network disk support
On 12/09/2010 04:09 AM, Daniel P. Berrange wrote:
> On Mon, Dec 06, 2010 at 04:24:09PM +0900, MORITA Kazutaka wrote:
>> This patch adds network disk support to libvirt/QEMU. The currently
>> supported protocols are nbd, rbd, and sheepdog. The XML syntax is like
>> this:
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> docs/schemas/domain.rng | 31 +++
Hmm; missing a corresponding update to docs/formatdomain.html.in to
document the new XML.
> ACK
NACK. After applying this patch, I get 'make check' failures:
TEST: vmx2xmltest
/bin/sh: line 5: 15285 Segmentation fault (core dumped)
abs_top_builddir=`cd '..'; pwd` abs_top_srcdir=`cd '..'; pwd`
abs_builddir=`cd '.'; pwd` abs_srcdir=`cd '.'; pwd` CONFIG_HEADER="`cd
'..'; pwd`/config.h"
PATH="$abs_top_builddir/src:$abs_top_builddir/daemon:$abs_top_builddir/tools:$PATH"
SHELL="/bin/sh"
LIBVIRT_DRIVER_DIR="/home/remote/eblake/libvirt/src/.libs" LC_ALL=C
${dir}$tst
FAIL: vmx2xmltest
TEST: xml2vmxtest
...!!...!.. 39 FAIL
I haven't looked into root cause. Kazutaka, would you mind preparing a v3?
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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] correct the signal's name
On 12/08/2010 11:41 PM, Wen Congyang wrote: > The signal's name is wrong... > > Signed-off-by: Wen Congyang > > --- > tools/console.c |8 > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/tools/console.c b/tools/console.c > index c2971cf..e126320 100644 > --- a/tools/console.c > +++ b/tools/console.c > @@ -365,10 +365,10 @@ int vshRunConsole(virDomainPtr dom, const char *devname) > } > > /* Restore original signal handlers */ > -signal(SIGQUIT, old_sigpipe); > -signal(SIGQUIT, old_sighup); > -signal(SIGQUIT, old_sigint); > -signal(SIGQUIT, old_sigterm); > +signal(SIGPIPE, old_sigpipe); > +signal(SIGHUP, old_sighup); > +signal(SIGINT, old_sigint); > +signal(SIGTERM, old_sigterm); > signal(SIGQUIT, old_sigquit); That's an embarrassing copy and paste, which has been there since 2007. ACK and pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 v6 3/4] Add a watchdog action `dump'
On 12/07/2010 11:19 PM, Hu Tao wrote: > `dump' watchdog action lets libvirtd to dump the guest when receives a > watchdog event (which probably means a guest crash) > > Currently only qemu is supported. > --- > src/conf/domain_conf.c |1 + > src/conf/domain_conf.h |1 + > src/qemu/libvirtd_qemu.aug |1 + > src/qemu/qemu.conf |5 ++ > src/qemu/qemu_conf.c| 16 ++- > src/qemu/qemu_conf.h|5 ++ > src/qemu/qemu_driver.c | 96 > +++ > src/qemu/test_libvirtd_qemu.aug |2 + > 8 files changed, 126 insertions(+), 1 deletions(-) Needed a bit of conflict resolution given other patches that have gone in since this was posted, as well as a tweak to pass 'make check' (test_libvirtd_qemu.aug was incomplete), but I've now pushed this patch since it had an ACK. As Daniel said, I'm omitting patch 4, and looking forward to his larger rewrite (which I did promise to review: https://www.redhat.com/archives/libvir-list/2010-December/msg00073.html). However, as Justin pointed out, the new dump option still needs documentation and testing: - mention 'dump' in docs/formatdomain.html.in - update to docs/schemas/domain.rng to list dump as a valid option - add an .xml file somewhere under tests/ that exercises the new XML option -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 v6 2/4] Add a new function doCoreDump
On 12/09/2010 03:54 AM, Daniel P. Berrange wrote: > On Wed, Dec 08, 2010 at 02:19:12PM +0800, Hu Tao wrote: >> This patch prepares for the next patch. >> --- >> src/qemu/qemu_driver.c | 132 >> +++- >> 1 files changed, 74 insertions(+), 58 deletions(-) > > ACK Pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 v6 1/4] threadpool impl
On 12/07/2010 11:19 PM, Hu Tao wrote: I fixed your 'make syntax-check' problems: preprocessor_indentation cppi: src/util/threadpool.h: line 27: not properly indented cppi: src/util/threadpool.h: line 29: not properly indented maint.mk: incorrect preprocessor indentation > +++ b/src/libvirt_private.syms > @@ -898,3 +898,9 @@ virXPathStringLimit; > virXPathULong; > virXPathULongHex; > virXPathULongLong; > + > + > +# threadpool.h > +virThreadPoolNew; > +virThreadPoolFree; > +virThreadPoolSendJob; then sorted this (#threadpool.h section before threads.h, and sort lines within the section), and pushed. Thanks for your persistence; the end result is nice! -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] libvirt-guests output
On 12/09/2010 11:21 AM, Laurent Léonard wrote: > Hi, > > Here is an example of libvirt-guest output: > $Running guests on default URI: test-vm > $Suspending guests on default URI... > $Suspending test-vm: $done > > Why all the lines begin with "$" ? Because you aren't using bash. Bash supports $"" as a way to provide gettext translation of strings into the user's preferred location. Init scripts written for Fedora-based systems assume that /bin/sh is bash, and therefore that $"" is usable. Dash does not understand $"" (POSIX says it has unspecified behavior), and treats it as a literal '$' followed by a normal (untranslated) "". It's probably a nice goal to port libvirt-guests to non-bash /bin/sh, but it may take a lot more work; in particular, there are TONS of existing init scripts on Fedora that assume bash extensions, which libvirt-guests used as its starting point. And I'm not quite sure what the POSIX-compliant replacement for $"" string translation would be. There are two problems to solve - how to get xgettext to recognize a string that needs translation, and how to use gettext(1) (rather than bash's magic $"" automatically calling gettext(3)) to do the translation at runtime. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] Fix memory leak in virCommandRun()
On 12/08/2010 05:33 PM, Hu Tao wrote: >> From 9ce936ee395e967551e36598ff0650315ac7686e Mon Sep 17 00:00:00 2001 >> From: Eric Blake >> Date: Wed, 8 Dec 2010 08:03:29 -0700 >> Subject: [PATCH] command: avoid memory leak >> >> * src/util/command.c (virCommandRun): Fix yesterday's regression >> on logging, and avoid leaking log-only output captures. >> Reported by Hu Tao. > > ACK. Thanks; pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 v6 3/4] Add a watchdog action `dump'
On 09/12/2010, at 9:55 PM, Daniel P. Berrange wrote: > On Wed, Dec 08, 2010 at 02:19:17PM +0800, Hu Tao wrote: >> `dump' watchdog action lets libvirtd to dump the guest when receives a >> watchdog event (which probably means a guest crash) >> >> Currently only qemu is supported. >> --- >> src/conf/domain_conf.c |1 + >> src/conf/domain_conf.h |1 + >> src/qemu/libvirtd_qemu.aug |1 + >> src/qemu/qemu.conf |5 ++ >> src/qemu/qemu_conf.c| 16 ++- >> src/qemu/qemu_conf.h|5 ++ >> src/qemu/qemu_driver.c | 96 >> +++ >> src/qemu/test_libvirtd_qemu.aug |2 + >> 8 files changed, 126 insertions(+), 1 deletions(-) Doesn't seem to be any updates to the documentation with the patch, explaining what it does and how to use it? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Distinguish between domain shutdown and crash
> > When we get an EOF event on monitor connection, it may be a result of > > either crash or graceful shutdown. QEMU which supports async events > > (i.e., we are talking to it using JSON monitor) emits SHUTDOWN event on > > graceful shutdown. In case we don't get this event by the time monitor > > connection is closed, we assume the associated domain crashed. > > --- > > src/qemu/qemu_driver.c | 23 +++ > > 1 files changed, 23 insertions(+), 0 deletions(-) > > ACK, Thanks, pushed. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] tests: Add tests for network disks
On Tue, Dec 07, 2010 at 11:57:33AM -0800, Josh Durgin wrote: > > Signed-off-by: Josh Durgin > --- > tests/qemuargv2xmltest.c |3 ++ > .../qemuxml2argv-disk-drive-network-nbd.args |1 + > .../qemuxml2argv-disk-drive-network-nbd.xml| 32 ++ > .../qemuxml2argv-disk-drive-network-rbd.args |1 + > .../qemuxml2argv-disk-drive-network-rbd.xml| 34 > > .../qemuxml2argv-disk-drive-network-sheepdog.args |1 + > .../qemuxml2argv-disk-drive-network-sheepdog.xml | 32 ++ > tests/qemuxml2argvtest.c |6 +++ > 8 files changed, 110 insertions(+), 0 deletions(-) > create mode 100644 > tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args > create mode 100644 > tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml > create mode 100644 > tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args > create mode 100644 > tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml > create mode 100644 > tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args > create mode 100644 > tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Add RBD support and some network disk fixes
On Tue, Dec 07, 2010 at 11:56:34AM -0800, Josh Durgin wrote: > > Changes common to all network disks: > -Make source name optional in the domain schema, since NBD doesn't use it > -Add a hostName type to the domain schema, and use it instead of genericName, > which doesn't include . > -Don't leak host names or ports > -Set the source protocol in qemuParseCommandline > > Signed-off-by: Josh Durgin > --- > docs/schemas/domain.rng | 11 +++- > src/conf/domain_conf.c | 25 +++- > src/conf/domain_conf.h |1 + > src/qemu/qemu_conf.c| 143 > --- > 4 files changed, 165 insertions(+), 15 deletions(-) ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] add network disk support
On Mon, Dec 06, 2010 at 04:24:09PM +0900, MORITA Kazutaka wrote: > This patch adds network disk support to libvirt/QEMU. The currently > supported protocols are nbd, rbd, and sheepdog. The XML syntax is like > this: > > > > > > > > > > > > Signed-off-by: MORITA Kazutaka > --- > > Hi, > > Thanks for your comments, Daniel. Here is a fixed version. > > Changes from v1 to v2 are: > - check whether the XML input is valid or not more strictly > - fix memory leak in the error path > - add NULL check of the return value of strdup() > > Thanks, > > Kazutaka > > > docs/schemas/domain.rng | 31 +++ > src/conf/domain_conf.c | 95 +++- > src/conf/domain_conf.h | 20 > src/qemu/qemu_conf.c| 221 > ++- > 4 files changed, 358 insertions(+), 9 deletions(-) ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch v3] Add VMware Workstation and Player driver
On Thursday 09 December 2010 11:23:38 Daniel P. Berrange wrote:
> On Thu, Dec 09, 2010 at 11:05:50AM +0100, Jean-Baptiste Rouault wrote:
> > On Wednesday 08 December 2010 19:29:56 Daniel P. Berrange wrote:
> > > FYI, you can still get CPUs which are 32-bit only and have vmx/svm
> > > supported.
> >
> > Indeed, I didn't know there were 32 bits CPUs with virtualization
> > extensions. Would it be ok to check for the "lm" CPU flag to be certain
> > that the host CPU is a 64bit one ?
>
> You really want to check what the OS is running, not what the CPU is,
> because you can put a 32-bit OS on a 64-bit CPU. Since VMware only
> copes with x86 platforms you can use
>
> STREQ(utsname.machine, "x86_64") ? 64 : 32
As far as I know, it is possible to run 64-bit guests on a 32-bit host OS if
there is a 64-bit CPU with vmx/svm so I think we don't care what the OS is in
that case, don't we ?
> > > > +static const char *
> > > > +vmwareGetType(virConnectPtr conn)
> > > > +{
> > > > +struct vmware_driver *driver = conn->privateData;
> > > > +int type;
> > > > +
> > > > +type = driver->type;
> > > > +return type == TYPE_PLAYER ? "vmware player" : "vmware
> > > > workstation"; +}
> > >
> > > This should just be returning the same string that's
> > > in the type field of the virDriverPtr struct that
> > > was registered.
> >
> > Do you mean the "name" field of the _virDriver struct ?
>
> Yes.
Ok, I thought the connectGetType() API was there to give more information
than the name field if relevant because virConnectGetType() returns
driver->name when the type function isn't available.
Jean-Baptiste
--
Jean-Baptiste ROUAULT
Ingénieur R&D - Diateam : Architectes de l'information
Phone : +33 (0)9 53 16 02 70 Fax : +33 (0)2 98 050 051
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Distinguish between domain shutdown and crash
On Thu, Dec 09, 2010 at 11:51:09AM +0100, Jiri Denemark wrote: > When we get an EOF event on monitor connection, it may be a result of > either crash or graceful shutdown. QEMU which supports async events > (i.e., we are talking to it using JSON monitor) emits SHUTDOWN event on > graceful shutdown. In case we don't get this event by the time monitor > connection is closed, we assume the associated domain crashed. > --- > src/qemu/qemu_driver.c | 23 +++ > 1 files changed, 23 insertions(+), 0 deletions(-) ACK, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 1/4] threadpool impl
On Wed, Dec 08, 2010 at 02:19:06PM +0800, Hu Tao wrote: > * src/util/threadpool.c, src/util/threadpool.h: Thread pool > implementation > * src/Makefile.am: Build thread pool > * src/libvirt_private.syms: Export public functions > --- > cfg.mk |1 + > src/Makefile.am |1 + > src/libvirt_private.syms |6 + > src/util/threadpool.c| 231 > ++ > src/util/threadpool.h| 48 ++ > 5 files changed, 287 insertions(+), 0 deletions(-) > create mode 100644 src/util/threadpool.c > create mode 100644 src/util/threadpool.h ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 4/4] Using threadpool API to manage qemud worker
On Wed, Dec 08, 2010 at 02:19:23PM +0800, Hu Tao wrote: > --- > daemon/libvirtd.c | 187 > - > daemon/libvirtd.h | 16 + > 2 files changed, 30 insertions(+), 173 deletions(-) While there is nothing wrong with this patch, I'd prefer if we didn't apply this one. I've got a huge patch series pending which basically deletes all the thread & RPC code from the libvirtd daemon, and replaces it with calls to a new set of libvirt internal RPC APIs. These new APIs of course make use of your thread pool implementation, so the net result will be the same. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 3/4] Add a watchdog action `dump'
On Wed, Dec 08, 2010 at 02:19:17PM +0800, Hu Tao wrote: > `dump' watchdog action lets libvirtd to dump the guest when receives a > watchdog event (which probably means a guest crash) > > Currently only qemu is supported. > --- > src/conf/domain_conf.c |1 + > src/conf/domain_conf.h |1 + > src/qemu/libvirtd_qemu.aug |1 + > src/qemu/qemu.conf |5 ++ > src/qemu/qemu_conf.c| 16 ++- > src/qemu/qemu_conf.h|5 ++ > src/qemu/qemu_driver.c | 96 > +++ > src/qemu/test_libvirtd_qemu.aug |2 + > 8 files changed, 126 insertions(+), 1 deletions(-) ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 2/4] Add a new function doCoreDump
On Wed, Dec 08, 2010 at 02:19:12PM +0800, Hu Tao wrote: > This patch prepares for the next patch. > --- > src/qemu/qemu_driver.c | 132 > +++- > 1 files changed, 74 insertions(+), 58 deletions(-) ACK Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Distinguish between domain shutdown and crash
When we get an EOF event on monitor connection, it may be a result of
either crash or graceful shutdown. QEMU which supports async events
(i.e., we are talking to it using JSON monitor) emits SHUTDOWN event on
graceful shutdown. In case we don't get this event by the time monitor
connection is closed, we assume the associated domain crashed.
---
src/qemu/qemu_driver.c | 23 +++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4e65f3b..ab42a6d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -129,6 +129,7 @@ struct _qemuDomainObjPrivate {
virDomainChrDefPtr monConfig;
int monJSON;
int monitor_warned;
+bool gotShutdown;
int nvcpupids;
int *vcpupids;
@@ -919,11 +920,19 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
int hasError) {
struct qemud_driver *driver = qemu_driver;
virDomainEventPtr event = NULL;
+qemuDomainObjPrivatePtr priv;
VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name);
virDomainObjLock(vm);
+priv = vm->privateData;
+if (!hasError && priv->monJSON && !priv->gotShutdown) {
+VIR_DEBUG("Monitor connection to '%s' closed without SHUTDOWN event; "
+ "assuming the domain crashed", vm->def->name);
+hasError = 1;
+}
+
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
hasError ?
@@ -1120,6 +1129,18 @@ qemuHandleDomainReset(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
static int
+qemuHandleDomainShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+ virDomainObjPtr vm)
+{
+virDomainObjLock(vm);
+((qemuDomainObjPrivatePtr) vm->privateData)->gotShutdown = true;
+virDomainObjUnlock(vm);
+
+return 0;
+}
+
+
+static int
qemuHandleDomainStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virDomainObjPtr vm)
{
@@ -1382,6 +1403,7 @@ static qemuMonitorCallbacks monitorCallbacks = {
.destroy = qemuHandleMonitorDestroy,
.eofNotify = qemuHandleMonitorEOF,
.diskSecretLookup = findVolumeQcowPassphrase,
+.domainShutdown = qemuHandleDomainShutdown,
.domainStop = qemuHandleDomainStop,
.domainReset = qemuHandleDomainReset,
.domainRTCChange = qemuHandleDomainRTCChange,
@@ -3997,6 +4019,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
priv->monJSON = 0;
priv->monitor_warned = 0;
+priv->gotShutdown = false;
if ((ret = virFileDeletePid(driver->stateDir, vm->def->name)) != 0) {
virReportSystemError(ret,
--
1.7.3.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch v3] Add VMware Workstation and Player driver
On Thu, Dec 09, 2010 at 11:05:50AM +0100, Jean-Baptiste Rouault wrote:
> On Wednesday 08 December 2010 19:29:56 Daniel P. Berrange wrote:
> >
> > FYI, you can still get CPUs which are 32-bit only and have vmx/svm
> > supported.
>
> Indeed, I didn't know there were 32 bits CPUs with virtualization extensions.
> Would it be ok to check for the "lm" CPU flag to be certain that the host CPU
> is a 64bit one ?
You really want to check what the OS is running, not what the CPU is,
because you can put a 32-bit OS on a 64-bit CPU. Since VMware only
copes with x86 platforms you can use
STREQ(utsname.machine, "x86_64") ? 64 : 32
>
> > > +
> > > +//vmrun list only reports running vms
> > > +vm->state = VIR_DOMAIN_RUNNING;
> > > +vm->def->id = driver->nextvmid++;
> > > +vm->persistent = 1;
> >
> > The VM ID is intended to be stable for the lifetime of a VM. It
> > seems like this could be unstable, depending on the order in
> > which vmrun -T returns the list. Is there any way to find a
> > more stable ID, even if it means using the VM's UNIX PID ?
>
> I guess I could parse the first line of the VM log (file vmware.log in the
> vmx
> directory) to get the PID.
>
> > > +static const char *
> > > +vmwareGetType(virConnectPtr conn)
> > > +{
> > > +struct vmware_driver *driver = conn->privateData;
> > > +int type;
> > > +
> > > +type = driver->type;
> > > +return type == TYPE_PLAYER ? "vmware player" : "vmware workstation";
> > > +}
> >
> > This should just be returning the same string that's
> > in the type field of the virDriverPtr struct that
> > was registered.
>
> Do you mean the "name" field of the _virDriver struct ?
Yes.
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch v3] Add VMware Workstation and Player driver
On Wednesday 08 December 2010 19:29:56 Daniel P. Berrange wrote:
>
> FYI, you can still get CPUs which are 32-bit only and have vmx/svm
> supported.
Indeed, I didn't know there were 32 bits CPUs with virtualization extensions.
Would it be ok to check for the "lm" CPU flag to be certain that the host CPU
is a 64bit one ?
> > +
> > +//vmrun list only reports running vms
> > +vm->state = VIR_DOMAIN_RUNNING;
> > +vm->def->id = driver->nextvmid++;
> > +vm->persistent = 1;
>
> The VM ID is intended to be stable for the lifetime of a VM. It
> seems like this could be unstable, depending on the order in
> which vmrun -T returns the list. Is there any way to find a
> more stable ID, even if it means using the VM's UNIX PID ?
I guess I could parse the first line of the VM log (file vmware.log in the vmx
directory) to get the PID.
> > +static const char *
> > +vmwareGetType(virConnectPtr conn)
> > +{
> > +struct vmware_driver *driver = conn->privateData;
> > +int type;
> > +
> > +type = driver->type;
> > +return type == TYPE_PLAYER ? "vmware player" : "vmware workstation";
> > +}
>
> This should just be returning the same string that's
> in the type field of the virDriverPtr struct that
> was registered.
Do you mean the "name" field of the _virDriver struct ?
Regards,
Jean-Baptiste
--
Jean-Baptiste ROUAULT
Ingénieur R&D - Diateam : Architectes de l'information
Phone : +33 (0)9 53 16 02 70 Fax : +33 (0)2 98 050 051
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
