[Libvir] fix SYNOPSIS of virsh --help schedinfo

2007-08-13 Thread Atsushi SAKAI
fix SYNOPSIS of virsh --help schedinfo.
from sched to schedinfo

Thanks
Atsushi SAKAI




fix_virsh_schedinfo_synopsys.patch
Description: Binary data
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] several typo fixes on man virsh

2007-08-13 Thread Atsushi SAKAI
Hi,

several typo fixes on man virsh
(uses ispell)

interract -> interact

suports -> supports

privledges -> privileges

synoposis -> synopsis

documetnation -> documentation

runable -> runnable

Ouput -> Output

guaruntee -> guarantee

processus -> processes  (2 typo)

Detatch -> Detach

dvices ->  devices

instanciated -> instantiated

Thanks
Atsushi SAKAI



virsh.pod.typo.patch
Description: Binary data
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH 0/7] QEMU/KVM save/restore support, take 3

2007-08-13 Thread Daniel Veillard
On Tue, Aug 14, 2007 at 03:19:24AM +0100, Daniel P. Berrange wrote:
> On Mon, Aug 13, 2007 at 02:17:03PM -0400, Jim Paris wrote:
> > Here's take 3 of the QEMU/KVM save/restore support.  Thanks for your
> > input.
> > 
> > Changes since last time:
> > 
> > - Remove escape sequence filtering, it's not necessary.
> > 
> > - Clean up stdin handling in virExec, use -1 to signify unused
> > 
> > - Add signal-safe read/write wrappers that handle EINTR and use them.
> > 
> > - Add version and padding to image header, and check version on restore.
> > 
> > - Include null-termination in XML data & length
> > 
> > - Show name of conflicting domain in error message
> > 
> > Everything seems to work well in my tests.  I've run into a few rare
> > cases where the migration doesn't work correctly (causing segfaults in
> > the guest, or kvm to crash), but it's not libvirt's fault, and libvirt
> > handles the failures well.
> 
> It all worked nicely for me too - exposed a bug in virt-manager too :-)
> Thanks again for coding & debugging this all

  Yes, I want to echo this, congrats and many thanks !

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH 0/7] QEMU/KVM save/restore support, take 3

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 02:17:03PM -0400, Jim Paris wrote:
> Here's take 3 of the QEMU/KVM save/restore support.  Thanks for your
> input.
> 
> Changes since last time:
> 
> - Remove escape sequence filtering, it's not necessary.
> 
> - Clean up stdin handling in virExec, use -1 to signify unused
> 
> - Add signal-safe read/write wrappers that handle EINTR and use them.
> 
> - Add version and padding to image header, and check version on restore.
> 
> - Include null-termination in XML data & length
> 
> - Show name of conflicting domain in error message
> 
> Everything seems to work well in my tests.  I've run into a few rare
> cases where the migration doesn't work correctly (causing segfaults in
> the guest, or kvm to crash), but it's not libvirt's fault, and libvirt
> handles the failures well.

It all worked nicely for me too - exposed a bug in virt-manager too :-)
Thanks again for coding & debugging this all

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Add KVM restore support using migration.

2007-08-13 Thread Daniel P. Berrange
Committed this too. Works very nicely - and wow it is fast to save & 
restore.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Add KVM save support using migration.

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 02:17:09PM -0400, Jim Paris wrote:
> The save file format consists of a header, XML for the domain,
> and the raw QEMU/KVM migration data stream.

I've comitted this 

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Add signal-safe read/write wrappers

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 02:17:08PM -0400, Jim Paris wrote:
> Adds saferead() and safewrite(), which are like read() and write()
> except that they retry in case of EINTR.

Good stuff, committed to CVS.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Add qemudEscapeShellArg for passing commandlines to qemu.

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 02:17:07PM -0400, Jim Paris wrote:
> Use this to escape a shell argument in a commandline passed to qemu.
> First we need to escape certain characters to get them through the
> qemu monitor interface.  On the shell side, the argument will be
> enclosed in single quotes, so the only character that needs special
> treatment is the single quote itself.

Looks good, committed to CVS.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Add migration support to QEMU startup.

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 02:17:06PM -0400, Jim Paris wrote:
> Adds new fields in qemu_vm structure.  vm->migrateFrom specifies the
> argument to "-incoming".  vm->stdin specifies the file descriptor to
> pass to virExec as stdin, which will be used for the "-incoming stdio"
> case.

Committed to CVS 

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Add option to pass stdin fd to virExec

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 02:17:05PM -0400, Jim Paris wrote:
> If non-negative, uses the supplied fd instead of /dev/null.
> Update callers accordingly.

Commited to CVS

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Fix issues with QEMU monitor interface.

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 02:17:04PM -0400, Jim Paris wrote:
> Due to the TTY layer, sending "\n" to the qemu monitor translates
> into "\r\n" when received.  This triggers a bug in older versions of
> QEMU (KVM <= 33) because the same command is executed twice, and
> still has problems with fixed QEMU because the "(qemu)" prompt is
> printed twice.  Switch all monitor commands to end with "\r" which
> avoids both issues.

Committed to CVS

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH 0/7] QEMU/KVM save/restore support, take 3

2007-08-13 Thread Jim Paris
Sorry, I'm still trying to learn these tools ... those subjects weren't 
numbered.

The order should be:

Subject: [PATCH 1/7] Fix issues with QEMU monitor interface.
Subject: [PATCH 2/7] Add option to pass stdin fd to virExec
Subject: [PATCH 3/7] Add migration support to QEMU startup.
Subject: [PATCH 4/7] Add qemudEscapeShellArg for passing commandlines to qemu.
Subject: [PATCH 5/7] Add signal-safe read/write wrappers
Subject: [PATCH 6/7] Add KVM save support using migration.
Subject: [PATCH 7/7] Add KVM restore support using migration.

-jim

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] [PATCH] Add signal-safe read/write wrappers

2007-08-13 Thread Jim Paris
Adds saferead() and safewrite(), which are like read() and write()
except that they retry in case of EINTR.

Signed-off-by: Jim Paris <[EMAIL PROTECTED]>
---
 src/util.c |   37 +
 src/util.h |2 ++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/src/util.c b/src/util.c
index 4f80eef..eb57859 100644
--- a/src/util.c
+++ b/src/util.c
@@ -189,3 +189,40 @@ virExecNonBlock(virConnectPtr conn,
 return(_virExec(conn, argv, retpid, infd, outfd, errfd, 1));
 }
 
+/* Like read(), but restarts after EINTR */
+int saferead(int fd, void *buf, size_t count)
+{
+   size_t nread = 0;
+   while (count > 0) { 
+   int r = read(fd, buf, count);
+   if (r < 0 && errno == EINTR)
+   continue;
+   if (r < 0)
+   return r;
+   if (r == 0)
+   return nread;
+   buf = (unsigned char *)buf + r;
+   count -= r;
+   nread += r;
+   }
+   return nread;
+}
+
+/* Like write(), but restarts after EINTR */
+ssize_t safewrite(int fd, const void *buf, size_t count)
+{
+   size_t nwritten = 0;
+   while (count > 0) {
+   int r = write(fd, buf, count);
+   if (r < 0 && errno == EINTR)
+   continue;
+   if (r < 0)
+   return r;
+   if (r == 0)
+   return nwritten;
+   buf = (unsigned char *)buf + r;
+   count -= r;
+   nwritten += r;
+   }
+   return nwritten;
+}
diff --git a/src/util.h b/src/util.h
index d11e6d9..f69fac8 100644
--- a/src/util.h
+++ b/src/util.h
@@ -24,3 +24,5 @@
 int virExec(virConnectPtr conn, char **argv, int *retpid, int infd, int 
*outfd, int *errfd);
 int virExecNonBlock(virConnectPtr conn, char **argv, int *retpid, int infd, 
int *outfd, int *errfd);
 
+int saferead(int fd, void *buf, size_t count);
+ssize_t safewrite(int fd, const void *buf, size_t count);
-- 
1.5.3.rc4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] [PATCH] Add migration support to QEMU startup.

2007-08-13 Thread Jim Paris
Adds new fields in qemu_vm structure.  vm->migrateFrom specifies the
argument to "-incoming".  vm->stdin specifies the file descriptor to
pass to virExec as stdin, which will be used for the "-incoming stdio"
case.

Signed-off-by: Jim Paris <[EMAIL PROTECTED]>
---
 src/qemu_conf.c   |   13 -
 src/qemu_conf.h   |2 ++
 src/qemu_driver.c |4 ++--
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index 79dd180..2bbd072 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1518,7 +1518,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
 (vm->def->os.initrd[0] ? 2 : 0) + /* initrd */
 (vm->def->os.cmdline[0] ? 2 : 0) + /* cmdline */
 (vm->def->graphicsType == QEMUD_GRAPHICS_VNC ? 2 :
- (vm->def->graphicsType == QEMUD_GRAPHICS_SDL ? 0 : 1)); /* graphics */
+ (vm->def->graphicsType == QEMUD_GRAPHICS_SDL ? 0 : 1)) + /* graphics 
*/
+(vm->migrateFrom[0] ? 3 : 0); /* migrateFrom */
 
 snprintf(memory, sizeof(memory), "%d", vm->def->memory/1024);
 snprintf(vcpus, sizeof(vcpus), "%d", vm->def->vcpus);
@@ -1767,6 +1768,15 @@ int qemudBuildCommandLine(virConnectPtr conn,
 /* SDL is the default. no args needed */
 }
 
+if (vm->migrateFrom[0]) {
+if (!((*argv)[++n] = strdup("-S")))
+goto no_memory;
+if (!((*argv)[++n] = strdup("-incoming")))
+goto no_memory;
+if (!((*argv)[++n] = strdup(vm->migrateFrom)))
+goto no_memory;
+}
+
 (*argv)[++n] = NULL;
 
 return 0;
@@ -1884,6 +1894,7 @@ qemudAssignVMDef(virConnectPtr conn,
 return NULL;
 }
 
+vm->stdin = -1;
 vm->stdout = -1;
 vm->stderr = -1;
 vm->monitor = -1;
diff --git a/src/qemu_conf.h b/src/qemu_conf.h
index 60a38b7..4a9b1ae 100644
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -199,6 +199,7 @@ struct qemud_vm_def {
 
 /* Guest VM runtime state */
 struct qemud_vm {
+int stdin;
 int stdout;
 int stderr;
 int monitor;
@@ -212,6 +213,7 @@ struct qemud_vm {
 
 char configFile[PATH_MAX];
 char autostartLink[PATH_MAX];
+char migrateFrom[PATH_MAX];
 
 struct qemud_vm_def *def; /* The current definition */
 struct qemud_vm_def *newDef; /* New definition to activate at shutdown */
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 15b94b8..e649060 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -656,9 +656,9 @@ static int qemudStartVMDaemon(virConnectPtr conn,
  errno, strerror(errno));
 
 if (virExecNonBlock(conn, argv, &vm->pid,
--1, &vm->stdout, &vm->stderr) == 0) {
+vm->stdin, &vm->stdout, &vm->stderr) == 0) {
 vm->id = driver->nextvmid++;
-vm->state = VIR_DOMAIN_RUNNING;
+vm->state = vm->migrateFrom[0] ? VIR_DOMAIN_PAUSED : 
VIR_DOMAIN_RUNNING;
 
 driver->ninactivevms--;
 driver->nactivevms++;
-- 
1.5.3.rc4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] [PATCH] Add KVM restore support using migration.

2007-08-13 Thread Jim Paris
Signed-off-by: Jim Paris <[EMAIL PROTECTED]>
---
 src/qemu_driver.c |  113 +++--
 1 files changed, 109 insertions(+), 4 deletions(-)

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index f2c4316..b0b6d62 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2041,10 +2041,115 @@ static int qemudDomainSave(virDomainPtr dom,
 
 
 static int qemudDomainRestore(virConnectPtr conn,
-   const char *path ATTRIBUTE_UNUSED) {
-/*struct qemud_driver *driver = (struct qemud_driver *)conn->privateData;*/
-qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "restore is 
not supported");
-return -1;
+   const char *path) {
+struct qemud_driver *driver = (struct qemud_driver *)conn->privateData;
+struct qemud_vm_def *def;
+struct qemud_vm *vm;
+int fd;
+char *xml;
+struct qemud_save_header header;
+
+/* Verify the header and read the XML */
+if ((fd = open(path, O_RDONLY)) < 0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "cannot read domain image");
+return -1;
+}
+
+if (saferead(fd, &header, sizeof(header)) != sizeof(header)) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to read qemu header");
+close(fd);
+return -1;
+}
+
+if (memcmp(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)) != 0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "image magic is incorrect");
+close(fd);
+return -1;
+}
+
+if (header.version > QEMUD_SAVE_VERSION) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "image version is not supported (%d > %d)", 
+ header.version, QEMUD_SAVE_VERSION);
+close(fd);
+return -1;
+}
+
+if ((xml = (char *)malloc(header.xml_len)) == NULL) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "out of memory");
+close(fd);
+return -1;
+}
+
+if (saferead(fd, xml, header.xml_len) != header.xml_len) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to read XML");
+close(fd);
+free(xml);
+return -1;
+}
+
+/* Create a domain from this XML */
+if (!(def = qemudParseVMDef(conn, driver, xml, NULL))) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to parse XML");
+close(fd);
+free(xml);
+return -1;
+}
+free(xml);
+
+/* Ensure the name and UUID don't already exist in an active VM */
+vm = qemudFindVMByUUID(driver, def->uuid);
+if (!vm) vm = qemudFindVMByName(driver, def->name);
+if (vm && qemudIsActiveVM(vm)) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "domain is already active as '%s'", vm->def->name);
+close(fd);
+return -1;
+}
+
+if (!(vm = qemudAssignVMDef(conn, driver, def))) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to assign new VM");
+qemudFreeVMDef(def);
+close(fd);
+return -1;
+}
+
+/* Set the migration source and start it up. */
+snprintf(vm->migrateFrom, sizeof(vm->migrateFrom), "stdio");
+vm->stdin = fd;
+
+if (qemudStartVMDaemon(conn, driver, vm) < 0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to start VM");
+if (!vm->configFile[0])
+qemudRemoveInactiveVM(driver, vm);
+close(fd);
+return -1;
+}
+close(fd);
+vm->migrateFrom[0] = '\0';
+vm->stdin = -1;
+
+/* If it was running before, resume it now. */
+if (header.was_running) {
+char *info;
+if (qemudMonitorCommand(driver, vm, "cont\r", &info) < 0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to resume domain");
+return -1;
+}
+free(info);
+vm->state = VIR_DOMAIN_RUNNING;
+}
+
+return 0;
 }
 
 
-- 
1.5.3.rc4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] [PATCH] Add KVM save support using migration.

2007-08-13 Thread Jim Paris
The save file format consists of a header, XML for the domain,
and the raw QEMU/KVM migration data stream.

Signed-off-by: Jim Paris <[EMAIL PROTECTED]>
---
 src/qemu_driver.c |  112 ++--
 1 files changed, 107 insertions(+), 5 deletions(-)

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 8125622..f2c4316 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1921,20 +1921,122 @@ static char *qemudEscapeShellArg(const char *in)
 }
 
 
+#define QEMUD_SAVE_MAGIC "LibvirtQemudSave"
+#define QEMUD_SAVE_VERSION 1
+
+struct qemud_save_header {
+char magic[sizeof(QEMUD_SAVE_MAGIC)-1];
+int version;
+int xml_len;
+int was_running;
+int unused[16];
+};
+
 static int qemudDomainSave(virDomainPtr dom,
-const char *path ATTRIBUTE_UNUSED) {
+   const char *path) {
 struct qemud_driver *driver = (struct qemud_driver 
*)dom->conn->privateData;
 struct qemud_vm *vm = qemudFindVMByID(driver, dom->id);
+char *command, *info;
+int fd;
+char *safe_path;
+char *xml;
+struct qemud_save_header header;
+
+memset(&header, 0, sizeof(header));
+memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic));
+header.version = QEMUD_SAVE_VERSION;
+
 if (!vm) {
-qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "no 
domain with matching id %d", dom->id);
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+ "no domain with matching id %d", dom->id);
 return -1;
 }
+
 if (!qemudIsActiveVM(vm)) {
-qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, 
"domain is not running");
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "domain is not running");
 return -1;
 }
-qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "save is 
not supported");
-return -1;
+
+/* Pause */
+if (vm->state == VIR_DOMAIN_RUNNING) {
+header.was_running = 1;
+if (qemudDomainSuspend(dom) != 0) {
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to pause domain");
+return -1;
+}
+}
+
+/* Get XML for the domain */
+xml = qemudGenerateXML(dom->conn, driver, vm, vm->def, 0);
+if (!xml) {
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to get domain xml");
+return -1;
+}
+header.xml_len = strlen(xml) + 1;
+
+/* Write header to file, followed by XML */
+if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to create '%s'", path);
+free(xml);
+return -1;
+}
+
+if (safewrite(fd, &header, sizeof(header)) != sizeof(header)) {
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to write save header");
+close(fd);
+free(xml);
+return -1;
+}
+
+if (safewrite(fd, xml, header.xml_len) != header.xml_len) {
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "failed to write xml");
+close(fd);
+free(xml);
+return -1;
+}
+
+close(fd);
+free(xml);
+
+/* Migrate to file */
+safe_path = qemudEscapeShellArg(path);
+if (!safe_path) {
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "out of memory");
+return -1;
+}
+if (asprintf (&command, "migrate \"exec:"
+  "dd of='%s' oflag=append conv=notrunc 2>/dev/null"
+  "\"\r", safe_path) == -1) {
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "out of memory");
+free(safe_path);
+return -1;
+}
+free(safe_path);
+
+if (qemudMonitorCommand(driver, vm, command, &info) < 0) {
+qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "migrate operation failed");
+free(command);
+return -1;
+}
+
+free(info);
+free(command);
+
+/* Shut it down */
+qemudShutdownVMDaemon(dom->conn, driver, vm);
+if (!vm->configFile[0])
+qemudRemoveInactiveVM(driver, vm);
+
+return 0;
 }
 
 
-- 
1.5.3.rc4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] [PATCH 0/7] QEMU/KVM save/restore support, take 3

2007-08-13 Thread Jim Paris
Here's take 3 of the QEMU/KVM save/restore support.  Thanks for your
input.

Changes since last time:

- Remove escape sequence filtering, it's not necessary.

- Clean up stdin handling in virExec, use -1 to signify unused

- Add signal-safe read/write wrappers that handle EINTR and use them.

- Add version and padding to image header, and check version on restore.

- Include null-termination in XML data & length

- Show name of conflicting domain in error message

Everything seems to work well in my tests.  I've run into a few rare
cases where the migration doesn't work correctly (causing segfaults in
the guest, or kvm to crash), but it's not libvirt's fault, and libvirt
handles the failures well.
(I suspect it's related to
  http://article.gmane.org/gmane.comp.emulators.kvm.devel/5583)

Thanks,
-jim

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] [PATCH] Add qemudEscapeShellArg for passing commandlines to qemu.

2007-08-13 Thread Jim Paris
Use this to escape a shell argument in a commandline passed to qemu.
First we need to escape certain characters to get them through the
qemu monitor interface.  On the shell side, the argument will be
enclosed in single quotes, so the only character that needs special
treatment is the single quote itself.

Signed-off-by: Jim Paris <[EMAIL PROTECTED]>
---
 src/qemu_driver.c |   66 +
 1 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index e649060..8125622 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1855,6 +1855,72 @@ static int qemudDomainGetInfo(virDomainPtr dom,
 }
 
 
+static char *qemudEscapeShellArg(const char *in)
+{
+int len = 0;
+int i, j;
+char *out;
+
+/* To pass through the QEMU monitor, we need to use escape
+   sequences: \r, \n, \", \\
+
+   To pass through both QEMU + the shell, we need to escape
+   the single character ' as the five characters '\\''
+*/
+
+for (i = 0; in[i] != '\0'; i++) {
+switch(in[i]) {
+case '\r':
+case '\n':
+case '"':
+case '\\':
+len += 2;
+break;
+case '\'':
+len += 5;
+break;
+default:
+len += 1;
+break;
+}
+}
+
+if ((out = (char *)malloc(len + 1)) == NULL)
+return NULL;
+
+for (i = j = 0; in[i] != '\0'; i++) {
+switch(in[i]) {
+case '\r':
+out[j++] = '\\';
+out[j++] = 'r';
+break;
+case '\n':
+out[j++] = '\\';
+out[j++] = 'n';
+break;
+case '"':
+case '\\':
+out[j++] = '\\';
+out[j++] = in[i];
+break;
+case '\'':
+out[j++] = '\'';
+out[j++] = '\\';
+out[j++] = '\\';
+out[j++] = '\'';
+out[j++] = '\'';
+break;
+default:
+out[j++] = in[i];
+break;
+}
+}
+out[j] = '\0';
+
+return out;
+}
+
+
 static int qemudDomainSave(virDomainPtr dom,
 const char *path ATTRIBUTE_UNUSED) {
 struct qemud_driver *driver = (struct qemud_driver 
*)dom->conn->privateData;
-- 
1.5.3.rc4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] [PATCH] Add option to pass stdin fd to virExec

2007-08-13 Thread Jim Paris
If non-negative, uses the supplied fd instead of /dev/null.
Update callers accordingly.

Signed-off-by: Jim Paris <[EMAIL PROTECTED]>
---
 src/openvz_driver.c |4 ++--
 src/qemu_driver.c   |5 +++--
 src/util.c  |   12 ++--
 src/util.h  |4 ++--
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/openvz_driver.c b/src/openvz_driver.c
index 84d514c..b994fab 100644
--- a/src/openvz_driver.c
+++ b/src/openvz_driver.c
@@ -342,7 +342,7 @@ static int openvzListDomains(virConnectPtr conn, int *ids, 
int nids) {
 char buf[32];
 const char *cmd[] = {VZLIST, "-ovpsid", "-H" , NULL};
 
-ret = virExec(conn, (char **)cmd, &pid, &outfd, &errfd);
+ret = virExec(conn, (char **)cmd, &pid, -1, &outfd, &errfd);
 if(ret == -1) {
 error(conn, VIR_ERR_INTERNAL_ERROR, "Could not exec " VZLIST);
 return (int)NULL;
@@ -373,7 +373,7 @@ static int openvzListDefinedDomains(virConnectPtr conn,
 const char *cmd[] = {VZLIST, "-ovpsid", "-H", NULL};
 
 /* the -S options lists only stopped domains */
-ret = virExec(conn, (char **)cmd, &pid, &outfd, &errfd);
+ret = virExec(conn, (char **)cmd, &pid, -1, &outfd, &errfd);
 if(ret == -1) {
 error(conn, VIR_ERR_INTERNAL_ERROR, "Could not exec " VZLIST);
 return (int)NULL;
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index e13e6a3..15b94b8 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -655,7 +655,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
 qemudLog(QEMUD_WARN, "Unable to write argv to logfile %d: %s",
  errno, strerror(errno));
 
-if (virExecNonBlock(conn, argv, &vm->pid, &vm->stdout, &vm->stderr) == 0) {
+if (virExecNonBlock(conn, argv, &vm->pid,
+-1, &vm->stdout, &vm->stderr) == 0) {
 vm->id = driver->nextvmid++;
 vm->state = VIR_DOMAIN_RUNNING;
 
@@ -912,7 +913,7 @@ dhcpStartDhcpDaemon(virConnectPtr conn,
 if (qemudBuildDnsmasqArgv(conn, network, &argv) < 0)
 return -1;
 
-ret = virExecNonBlock(conn, argv, &network->dnsmasqPid, NULL, NULL);
+ret = virExecNonBlock(conn, argv, &network->dnsmasqPid, -1, NULL, NULL);
 
 for (i = 0; argv[i]; i++)
 free(argv[i]);
diff --git a/src/util.c b/src/util.c
index f53cfd2..4f80eef 100644
--- a/src/util.c
+++ b/src/util.c
@@ -79,7 +79,7 @@ static int virSetNonBlock(int fd) {
 static int
 _virExec(virConnectPtr conn,
   char **argv,
-  int *retpid, int *outfd, int *errfd, int non_block) {
+  int *retpid, int infd, int *outfd, int *errfd, int non_block) {
 int pid, null;
 int pipeout[2] = {-1,-1};
 int pipeerr[2] = {-1,-1};
@@ -140,7 +140,7 @@ _virExec(virConnectPtr conn,
 if (pipeerr[0] > 0 && close(pipeerr[0]) < 0)
 _exit(1);
 
-if (dup2(null, STDIN_FILENO) < 0)
+if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0)
 _exit(1);
 if (dup2(pipeout[1] > 0 ? pipeout[1] : null, STDOUT_FILENO) < 0)
 _exit(1);
@@ -176,16 +176,16 @@ _virExec(virConnectPtr conn,
 int
 virExec(virConnectPtr conn,
   char **argv,
-  int *retpid, int *outfd, int *errfd) {
+  int *retpid, int infd, int *outfd, int *errfd) {
 
-return(_virExec(conn, argv, retpid, outfd, errfd, 0));
+return(_virExec(conn, argv, retpid, infd, outfd, errfd, 0));
 }
 
 int
 virExecNonBlock(virConnectPtr conn,
   char **argv,
-  int *retpid, int *outfd, int *errfd) {
+  int *retpid, int infd, int *outfd, int *errfd) {
 
-return(_virExec(conn, argv, retpid, outfd, errfd, 1));
+return(_virExec(conn, argv, retpid, infd, outfd, errfd, 1));
 }
 
diff --git a/src/util.h b/src/util.h
index 5b84043..d11e6d9 100644
--- a/src/util.h
+++ b/src/util.h
@@ -21,6 +21,6 @@
  * File created Jul 18, 2007 - Shuveb Hussain <[EMAIL PROTECTED]>
  */
 
-int virExec(virConnectPtr conn, char **argv, int *retpid, int *outfd, int 
*errfd);
-int virExecNonBlock(virConnectPtr conn, char **argv, int *retpid, int *outfd, 
int *errfd);
+int virExec(virConnectPtr conn, char **argv, int *retpid, int infd, int 
*outfd, int *errfd);
+int virExecNonBlock(virConnectPtr conn, char **argv, int *retpid, int infd, 
int *outfd, int *errfd);
 
-- 
1.5.3.rc4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] [PATCH] Fix issues with QEMU monitor interface.

2007-08-13 Thread Jim Paris
Due to the TTY layer, sending "\n" to the qemu monitor translates
into "\r\n" when received.  This triggers a bug in older versions of
QEMU (KVM <= 33) because the same command is executed twice, and
still has problems with fixed QEMU because the "(qemu)" prompt is
printed twice.  Switch all monitor commands to end with "\r" which
avoids both issues.

Signed-off-by: Jim Paris <[EMAIL PROTECTED]>
---
 src/qemu_driver.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index b05c3f6..e13e6a3 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1755,7 +1755,7 @@ static int qemudDomainSuspend(virDomainPtr dom) {
 if (vm->state == VIR_DOMAIN_PAUSED)
 return 0;
 
-if (qemudMonitorCommand(driver, vm, "stop\n", &info) < 0) {
+if (qemudMonitorCommand(driver, vm, "stop\r", &info) < 0) {
 qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, 
"suspend operation failed");
 return -1;
 }
@@ -1780,7 +1780,7 @@ static int qemudDomainResume(virDomainPtr dom) {
 }
 if (vm->state == VIR_DOMAIN_RUNNING)
 return 0;
-if (qemudMonitorCommand(driver, vm, "cont\n", &info) < 0) {
+if (qemudMonitorCommand(driver, vm, "cont\r", &info) < 0) {
 qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, 
"resume operation failed");
 return -1;
 }
-- 
1.5.3.rc4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 7/7] Add KVM restore support using migration.

2007-08-13 Thread Daniel Veillard
On Sun, Aug 12, 2007 at 07:11:39PM -0400, Jim Paris wrote:
> 
> Signed-off-by: Jim Paris <[EMAIL PROTECTED]>

> +if ((xml = (char *)malloc(header.xml_len + 1)) == NULL) {
> +qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> + "out of memory");
> +close(fd);
> +return -1;
> +}
> +
> +if (read(fd, xml, header.xml_len) != header.xml_len) {
> +qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> + "failed to read XML");
> +close(fd);
> +free(xml);
> +return -1;
> +}
> +xml[header.xml_len] = '\0';

  I would rather save xml_len to include the trailing 0 as part of the save
and simplify the + 1. For example it would be legal to have an XML description
done in UTF-16, where adding a single trailing 0 byte would not be sufficient,
better consider the lenght to be the full data, NUL character included.
  Oh and wrapping the read() here too, as suggested in previous patch.

> +vm = qemudFindVMByUUID(driver, def->uuid);
> +if (!vm) vm = qemudFindVMByName(driver, def->name);
> +if (vm && qemudIsActiveVM(vm)) {
> +qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> + "domain to restore is already active");

  Would be great to have the name printed here in the error message, allowing
to take corrective action or at least some easy analysis

  thanks,

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 6/7] Add KVM save support using migration.

2007-08-13 Thread Daniel Veillard
On Sun, Aug 12, 2007 at 07:11:38PM -0400, Jim Paris wrote:
> The save file format consists of a header, XML for the domain,
> and the raw QEMU/KVM migration data stream.
> -static int qemudDomainSave(virDomainPtr dom,
> -const char *path ATTRIBUTE_UNUSED) {
> +#define QEMUD_SAVE_MAGIC "LibvirtQemudSave"
> +struct qemud_save_header {
> +char magic[sizeof(QEMUD_SAVE_MAGIC)-1];


  I suggest to add an "int version" field here to be able to extend the
format.
> +int xml_len;
> +int was_running;

  and "int unused[16];"

With those 2 we should be able to cope with backward compatibility on
saved domain even if we don't know yet what may be needed. At worse it's
a few bytes lost in a very big file, at best it's a life saver.

> +memset(&header, 0, sizeof(header));
> +memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic));

#define QEMU_SAVE_VERSION 1 somewhere and then 

   header.version = QEMU_SAVE_VERSION;

> +header.xml_len = strlen(xml);

   maybe + 1 to account for the trailing 0

> +if (write(fd, &header, sizeof(header)) != sizeof(header)) {
> +qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> + "failed to write save header");
> +close(fd);
> +free(xml);
> +return -1;
> +}

  I suggest to define or reuse existing safe write and read routines which
handle interruped calls with EAGAIN.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 3/7] Add option to pass stdin fd to virExec

2007-08-13 Thread Jim Paris
Daniel P. Berrange wrote:
> > > If nonzero, uses the supplied fd instead of /dev/null.
> > > Update callers accordingly.
> > 
> >   Looks fine to me, we already have stdout and stderr, it's sensible. +1
> 
> Looks OK, but shouldn't we use -1 as the magic value here, since 0 is also
> a valid file descriptor.

Yeah, that was me taking advantage of the fact that the qemu_vm
structure is initialized to 0.  I'll change that to -1 and make sure
all qemu_vm users initialize stdinFd correctly.

-jim

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 2/7] Fix issues with QEMU monitor interface.

2007-08-13 Thread Jim Paris
Daniel Veillard wrote:
> > +/* Copy data, skipping 3-byte escape sequences */
> > +for (i = 0; i < got; i++) {
> > +if (data[i] == '\033')
> > +skip = 3;
> > +if (skip)
> > +skip--;
> > +else
> > +buf[size++] = data[i];
> > +}
> > +buf[size] = '\0';
> >  }
> 
>   It seems that if for some reason you do a partial read on the QEmu
> console descriptor ending in the middle of the escape command you may
> have a problem.

It should be OK.  Partial reads are why I'm setting using the "skip"
variable which is persistent across read() calls.  Any time we see
'\033' we'll always skip three bytes from qemu.  Note that partial
reads across qemuMonitorCommand calls doesn't really matter, because
we really just care about finding the next prompt anyway, and there
shouldn't be any data received between the prompt and the execution of
the next command.


Daniel P. Berrange wrote:
> We're reading from a Psuedo-TTY which is line buffered, so I think the OS 
> should guarentee that we can read a whole lines worth of data without 
> getting EAGAIN.

QEMU disables line buffering when it initializes the pty.


> It looks sane to me - I had no idea QEMU was sending this escape
> sequences.

It comes from qemu's readline.c:term_update and is a bit of a pain.

... Actually, on closer inspection, I think this patch might be
misguided.  The only case where you should get escape sequences after
the "\n" but before the "(qemu)" is when you are sending CRLF to a
version of KVM that's supposed to be better at handling it -- it turns
out my kvm patch was incomplete and didn't reset all of the input
state.

When monitor commands are terminated with "\r" rather than "\n",
this should never occur.  And so filtering escape sequences should be
unnecessary, as they should only show up on the echoed command line.

There were also some bugs in my libvirt patches (a merge error left
two commands terminated with "\n", and I left some debug output).
I'll fix things up and send an updated series.

-jim

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 3/7] Add option to pass stdin fd to virExec

2007-08-13 Thread Daniel Veillard
On Mon, Aug 13, 2007 at 12:55:01PM +0100, Daniel P. Berrange wrote:
> On Mon, Aug 13, 2007 at 06:01:42AM -0400, Daniel Veillard wrote:
> > On Sun, Aug 12, 2007 at 07:11:35PM -0400, Jim Paris wrote:
> > > If nonzero, uses the supplied fd instead of /dev/null.
> > > Update callers accordingly.
> > 
> >   Looks fine to me, we already have stdout and stderr, it's sensible. +1
> 
> Looks OK, but shouldn't we use -1 as the magic value here, since 0 is also
> a valid file descriptor.

  ohh, right, good catch ! +1 with that change,

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 2/7] Fix issues with QEMU monitor interface.

2007-08-13 Thread Daniel Veillard
On Mon, Aug 13, 2007 at 12:54:05PM +0100, Daniel P. Berrange wrote:
> On Mon, Aug 13, 2007 at 05:59:15AM -0400, Daniel Veillard wrote:
> > On Sun, Aug 12, 2007 at 07:11:34PM -0400, Jim Paris wrote:
> > > Due to the TTY layer, sending "\n" to the qemu monitor translates
> > > into "\r\n" when received.  This triggers a bug in older versions of
> > > QEMU (KVM <= 33) because the same command is executed twice, and
> > > still has problems with fixed QEMU because the "(qemu)" prompt is
> > > printed twice.  Switch all monitor commands to end with "\r" which
> > > avoids both issues.
> > > 
> > > The QEMU monitor sends frequent terminal escape sequences, typically
> > > \033[D and \033[K.  At times, these interfere with the prompt
> > > detection when they get sent between "\n" and "(qemu) ".  Fix the
> > > issue by filtering out these sequences when they are received.
> > 
> >   I think DanP can better comment on the QEmu interaction than me,
> > but the patch looks simple and clean except:
> 
> It looks sane to me - I had no idea QEMU was sending this escape sequences.
> 
> > 
> > > @@ -1333,14 +1335,23 @@ static int qemudMonitorCommand(struct 
> > > qemud_driver *driver ATTRIBUTE_UNUSED,
> > >  return -1;
> > >  }
> > >  buf = b;
> > > -memmove(buf+size, data, got);
> > > -buf[size+got] = '\0';
> > > -size += got;
> > > +
> > > +/* Copy data, skipping 3-byte escape sequences */
> > > +for (i = 0; i < got; i++) {
> > > +if (data[i] == '\033')
> > > +skip = 3;
> > > +if (skip)
> > > +skip--;
> > > +else
> > > +buf[size++] = data[i];
> > > +}
> > > +buf[size] = '\0';
> > >  }
> > 
> >   It seems that if for some reason you do a partial read on the QEmu
> > console descriptor ending in the middle of the escape command you may
> > have a problem. But it's possible that the way reads are done, and input
> > is chunked garantees that this won't happen, DanP can probably confirm 
> > it's just fine :-)
> 
> We're reading from a Psuedo-TTY which is line buffered, so I think the OS 
> should guarentee that we can read a whole lines worth of data without 
> getting EAGAIN.

  +1 then

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Block device and network stats

2007-08-13 Thread Daniel Veillard
On Mon, Aug 13, 2007 at 12:45:39PM +0100, Daniel P. Berrange wrote:
> On Mon, Aug 13, 2007 at 05:45:27AM -0400, Daniel Veillard wrote:
> > On Fri, Aug 10, 2007 at 03:35:08PM +0100, Richard W.M. Jones wrote:
> > > This is the implementation (currently Xen, local only).
> > 
> >   Thanks !
> > 
> > > +++ include/libvirt/libvirt.h.in  10 Aug 2007 14:30:21 -
> > > @@ -14,6 +14,9 @@
> > >  #ifndef __VIR_VIRLIB_H__
> > >  #define __VIR_VIRLIB_H__
> > >  
> > > +#include 
> > > +#include 
> > > +
> > [...]
> > > +struct _virDomainBlockStats {
> > > +  int64_t rd_req;
> > 
> >   That's my only worry at the moment. stdint.h isn't really that portable,
> > we want to define an 64bits unsigned field, but we already use 
> >unsigned long long 
> > in libvirt.h . I would be tempted to rationalize this, either we think
> > (stdint.h/int64_t) is more portable or long long is the one, but I would
> > prefer if we picked one and stick with it at the API level.
> 
> I don't mind one way or the other - there's not really much to choose 
> between them - int64_t is POSIX, while long long is C99. So both are 
> 'standards'. They've both been available on Linux & Solaris for as long
> as I can remember.
> 
> # info gcc
> "5.8 Double-Word Integers
> 
> 
> ISO C99 supports data types for integers that are at least 64 bits wide,
> and as an extension GCC supports them in C89 mode and in C++.  Simply
> write `long long int' for a signed integer, or `unsigned long long int'
> for an unsigned integer. "
> 
> 
> # info inttypes.h
> "If an implementation provides integer types with width 64 that meet these
>  requirements, then the following types are required: int64_t uint64_t"
> 
> 
> Rock, paper, scissors. C99 wins!

  The only difference is that in some future (hopefully we will be retired
by then ;-) long long may actually end up being say 128bits on some hardware.
Honnestly I don't care, since we are already using unsigned long long in the
header then let's stick to it and use it for the statistics too.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 3/7] Add option to pass stdin fd to virExec

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 06:01:42AM -0400, Daniel Veillard wrote:
> On Sun, Aug 12, 2007 at 07:11:35PM -0400, Jim Paris wrote:
> > If nonzero, uses the supplied fd instead of /dev/null.
> > Update callers accordingly.
> 
>   Looks fine to me, we already have stdout and stderr, it's sensible. +1

Looks OK, but shouldn't we use -1 as the magic value here, since 0 is also
a valid file descriptor.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 2/7] Fix issues with QEMU monitor interface.

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 05:59:15AM -0400, Daniel Veillard wrote:
> On Sun, Aug 12, 2007 at 07:11:34PM -0400, Jim Paris wrote:
> > Due to the TTY layer, sending "\n" to the qemu monitor translates
> > into "\r\n" when received.  This triggers a bug in older versions of
> > QEMU (KVM <= 33) because the same command is executed twice, and
> > still has problems with fixed QEMU because the "(qemu)" prompt is
> > printed twice.  Switch all monitor commands to end with "\r" which
> > avoids both issues.
> > 
> > The QEMU monitor sends frequent terminal escape sequences, typically
> > \033[D and \033[K.  At times, these interfere with the prompt
> > detection when they get sent between "\n" and "(qemu) ".  Fix the
> > issue by filtering out these sequences when they are received.
> 
>   I think DanP can better comment on the QEmu interaction than me,
> but the patch looks simple and clean except:

It looks sane to me - I had no idea QEMU was sending this escape sequences.

> 
> > @@ -1333,14 +1335,23 @@ static int qemudMonitorCommand(struct qemud_driver 
> > *driver ATTRIBUTE_UNUSED,
> >  return -1;
> >  }
> >  buf = b;
> > -memmove(buf+size, data, got);
> > -buf[size+got] = '\0';
> > -size += got;
> > +
> > +/* Copy data, skipping 3-byte escape sequences */
> > +for (i = 0; i < got; i++) {
> > +if (data[i] == '\033')
> > +skip = 3;
> > +if (skip)
> > +skip--;
> > +else
> > +buf[size++] = data[i];
> > +}
> > +buf[size] = '\0';
> >  }
> 
>   It seems that if for some reason you do a partial read on the QEmu
> console descriptor ending in the middle of the escape command you may
> have a problem. But it's possible that the way reads are done, and input
> is chunked garantees that this won't happen, DanP can probably confirm 
> it's just fine :-)

We're reading from a Psuedo-TTY which is line buffered, so I think the OS 
should guarentee that we can read a whole lines worth of data without 
getting EAGAIN.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Block device and network stats

2007-08-13 Thread Daniel P. Berrange
On Mon, Aug 13, 2007 at 05:45:27AM -0400, Daniel Veillard wrote:
> On Fri, Aug 10, 2007 at 03:35:08PM +0100, Richard W.M. Jones wrote:
> > This is the implementation (currently Xen, local only).
> 
>   Thanks !
> 
> > +++ include/libvirt/libvirt.h.in10 Aug 2007 14:30:21 -
> > @@ -14,6 +14,9 @@
> >  #ifndef __VIR_VIRLIB_H__
> >  #define __VIR_VIRLIB_H__
> >  
> > +#include 
> > +#include 
> > +
> [...]
> > +struct _virDomainBlockStats {
> > +  int64_t rd_req;
> 
>   That's my only worry at the moment. stdint.h isn't really that portable,
> we want to define an 64bits unsigned field, but we already use 
>unsigned long long 
> in libvirt.h . I would be tempted to rationalize this, either we think
> (stdint.h/int64_t) is more portable or long long is the one, but I would
> prefer if we picked one and stick with it at the API level.

I don't mind one way or the other - there's not really much to choose 
between them - int64_t is POSIX, while long long is C99. So both are 
'standards'. They've both been available on Linux & Solaris for as long
as I can remember.

# info gcc
"5.8 Double-Word Integers


ISO C99 supports data types for integers that are at least 64 bits wide,
and as an extension GCC supports them in C89 mode and in C++.  Simply
write `long long int' for a signed integer, or `unsigned long long int'
for an unsigned integer. "


# info inttypes.h
"If an implementation provides integer types with width 64 that meet these
 requirements, then the following types are required: int64_t uint64_t"


Rock, paper, scissors. C99 wins!

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 3/7] Add option to pass stdin fd to virExec

2007-08-13 Thread Daniel Veillard
On Sun, Aug 12, 2007 at 07:11:35PM -0400, Jim Paris wrote:
> If nonzero, uses the supplied fd instead of /dev/null.
> Update callers accordingly.

  Looks fine to me, we already have stdout and stderr, it's sensible. +1

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 2/7] Fix issues with QEMU monitor interface.

2007-08-13 Thread Daniel Veillard
On Sun, Aug 12, 2007 at 07:11:34PM -0400, Jim Paris wrote:
> Due to the TTY layer, sending "\n" to the qemu monitor translates
> into "\r\n" when received.  This triggers a bug in older versions of
> QEMU (KVM <= 33) because the same command is executed twice, and
> still has problems with fixed QEMU because the "(qemu)" prompt is
> printed twice.  Switch all monitor commands to end with "\r" which
> avoids both issues.
> 
> The QEMU monitor sends frequent terminal escape sequences, typically
> \033[D and \033[K.  At times, these interfere with the prompt
> detection when they get sent between "\n" and "(qemu) ".  Fix the
> issue by filtering out these sequences when they are received.

  I think DanP can better comment on the QEmu interaction than me,
but the patch looks simple and clean except:

> @@ -1333,14 +1335,23 @@ static int qemudMonitorCommand(struct qemud_driver 
> *driver ATTRIBUTE_UNUSED,
>  return -1;
>  }
>  buf = b;
> -memmove(buf+size, data, got);
> -buf[size+got] = '\0';
> -size += got;
> +
> +/* Copy data, skipping 3-byte escape sequences */
> +for (i = 0; i < got; i++) {
> +if (data[i] == '\033')
> +skip = 3;
> +if (skip)
> +skip--;
> +else
> +buf[size++] = data[i];
> +}
> +buf[size] = '\0';
>  }

  It seems that if for some reason you do a partial read on the QEmu
console descriptor ending in the middle of the escape command you may
have a problem. But it's possible that the way reads are done, and input
is chunked garantees that this won't happen, DanP can probably confirm 
it's just fine :-)

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] Re: [PATCH 1/7] Fix memory leak

2007-08-13 Thread Daniel Veillard
On Sun, Aug 12, 2007 at 07:11:33PM -0400, Jim Paris wrote:
> 
> Signed-off-by: Jim Paris <[EMAIL PROTECTED]>
> ---
>  src/qemu_driver.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 7c75d9c..b05c3f6 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -204,6 +204,7 @@ qemudStartup(void) {
>  qemudShutdown();
>  qemudAutostartConfigs(qemu_driver);
>  
> +free(base);
>  return 0;
>  
>   snprintf_error:

  Okay, uncontroversial bug, applied, will commit shortly,

thanks !

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Block device and network stats

2007-08-13 Thread Daniel Veillard
On Fri, Aug 10, 2007 at 03:35:08PM +0100, Richard W.M. Jones wrote:
> This is the implementation (currently Xen, local only).

  Thanks !

> +++ include/libvirt/libvirt.h.in  10 Aug 2007 14:30:21 -
> @@ -14,6 +14,9 @@
>  #ifndef __VIR_VIRLIB_H__
>  #define __VIR_VIRLIB_H__
>  
> +#include 
> +#include 
> +
[...]
> +struct _virDomainBlockStats {
> +  int64_t rd_req;

  That's my only worry at the moment. stdint.h isn't really that portable,
we want to define an 64bits unsigned field, but we already use 
   unsigned long long 
in libvirt.h . I would be tempted to rationalize this, either we think
(stdint.h/int64_t) is more portable or long long is the one, but I would
prefer if we picked one and stick with it at the API level.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list