12:38 a.m.
On Wed Apr 07 I sent an email with a patch titled Live Migration with
non-shared storage for kvm. What is the status on the patch?
Kenneth Nagin
Ph: +972-4-8296227
Cell: 054-6976227
Fx: +972-4- 8296113
"Do not look back in anger, or forward in fear, but around in awareness."
--James Thurber
11:10 a.m.
On 04/19/2010 01:38 AM, Kenneth Nagin wrote:
On Wed Apr 07 I sent an email with a patch titled Live Migration with
non-shared storage for kvm. What is the status on the patch?
If it didn't get an adequate response, it's probably a good idea to
rebase the patch against latest code, and resend to the list: people may
have deleted the old message.
- Cole
2:27 a.m.
Cole Robinson <crobinso(a)redhat.com> wrote on 20/04/2010
19:10:10:
From: Cole Robinson <crobinso(a)redhat.com>
To: Kenneth Nagin/Haifa/IBM@IBMIL
Cc: list libvirt <libvir-list(a)redhat.com>
Date: 20/04/2010 19:10
Subject: Re: [libvirt] Live Migration with non-shared storage for kvm
On 04/19/2010 01:38 AM, Kenneth Nagin wrote:
>
> On Wed Apr 07 I sent an email with a patch titled Live Migration with
> non-shared storage for kvm. What is the status on the patch?
>
If it didn't get an adequate response, it's probably a good idea to
rebase the patch against latest code, and resend to the list: people may
have deleted the old message.
- Cole
This is the second time I have been asked to rebase. I submitted the same
patch in February. Daniel Veillard asked me to rebase that patch. I got
very busy so I delayed submitting the patch until now. If necessary I'll
submit again, but I'd like some assurance that I'm not just wasting my and
your time.
-- Kenneth Nagin
8:57 a.m.
On 04/21/2010 03:27 AM, Kenneth Nagin wrote:
> Cole Robinson <crobinso(a)redhat.com> wrote on 20/04/2010 19:10:10:
> From: Cole Robinson <crobinso(a)redhat.com>
> To: Kenneth Nagin/Haifa/IBM@IBMIL
> Cc: list libvirt <libvir-list(a)redhat.com>
> Date: 20/04/2010 19:10
> Subject: Re: [libvirt] Live Migration with non-shared storage for kvm
>
> On 04/19/2010 01:38 AM, Kenneth Nagin wrote:
>>
>> On Wed Apr 07 I sent an email with a patch titled Live Migration with
>> non-shared storage for kvm. What is the status on the patch?
>>
>
> If it didn't get an adequate response, it's probably a good idea to
> rebase the patch against latest code, and resend to the list: people may
> have deleted the old message.
>
> - Cole
>
This is the second time I have been asked to rebase. I submitted the same
patch in February. Daniel Veillard asked me to rebase that patch. I got
very busy so I delayed submitting the patch until now. If necessary I'll
submit again, but I'd like some assurance that I'm not just wasting my and
your time.
If you rebase and resend the patch, I'll review it.
But seriously, everyone is always busy, and sometimes things slip
through the cracks. If in the future you want to call attention to a
forgotten patch, either rebase and resend with [RESEND] in the subject,
or reply to the original patch submission with a 'ping' or similar,
keeping the original mail intact so people don't have to check the
archives to find the mail you are talking about.
- Cole
10:06 a.m.
New subject: [libvirt] (Resend) Live Migration with non-shared storage for kvm
Support for live migration between hosts that do not share storage was
added to qemu-kvm release 0.12.1.
It supports two flags:
-b migration without shared storage with full disk copy
-i migration without shared storage with incremental copy (same base image
shared between source and destination).
In order to support kvm's live migration with non-shared storage I added
two migration flags that user can invoke:
VIR_MIGRATE_NON_SHARED_DISK = (1 << 6), /* migration with non-shared
storage with full disk copy */
VIR_MIGRATE_NON_SHARED_INC = (1 << 7), /* migration with non-shared
storage with incremental copy */
/* (same base image shared
between source and destination) */
Likewise I add complementary flags to virsh migrate: non_shared_disk and
non_shared_inc
As Daniel B. suggested I also added internal flags to be passed in the
"background" parameter to the qemu monitor:
typedef enum {
QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0,
QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with
non-shared storage with full disk copy */
QEMU_MONITOR_MIGRATE_NON_SHARED_INC = 1 << 2, /* migration with
non-shared storage with incremental copy */
QEMU_MONITOR_MIGRATION_FLAGS_LAST
};
I updated qemu_driver.c's doNativeMigrate and doTunnelMigrate to map the
external flags to the internal flags.
I updated qemu_monitor_text's qemuMonitorTextMigrate to map the internal
flags to the qemu monitor's command line flags.
Also, qemudDomainSave ends up calling qemuMonitorTextMigrate, but it didn't
support the external flags so I assumed that it was not relevant.
I tested the live migration without shared storage (both flags) for native
and p2p with and without tunnelling. I also verified that the fix doesn't
affect normal migration with shared storage.
Here is the diff patch file from the current git:
(See attached file: libvirt_migration_ns_100422.patch)
-- Kenneth Nagin
Cole Robinson <crobinso(a)redhat.com> wrote on 21/04/2010
16:57:29:
From: Cole Robinson <crobinso(a)redhat.com>
To: Kenneth Nagin/Haifa/IBM@IBMIL
Cc: Daniel Veillard <veillard(a)redhat.com>, "Daniel P. Berrange"
<berrange(a)redhat.com>, list libvirt <libvir-list(a)redhat.com>
Date: 21/04/2010 16:57
Subject: Re: [libvirt] Live Migration with non-shared storage for kvm
On 04/21/2010 03:27 AM, Kenneth Nagin wrote:
>
>> Cole Robinson <crobinso(a)redhat.com> wrote on 20/04/2010 19:10:10:
>
>> From: Cole Robinson <crobinso(a)redhat.com>
>> To: Kenneth Nagin/Haifa/IBM@IBMIL
>> Cc: list libvirt <libvir-list(a)redhat.com>
>> Date: 20/04/2010 19:10
>> Subject: Re: [libvirt] Live Migration with non-shared storage for kvm
>>
>> On 04/19/2010 01:38 AM, Kenneth Nagin wrote:
>>>
>>> On Wed Apr 07 I sent an email with a patch titled Live Migration
with
>>> non-shared storage for kvm. What is the status on the
patch?
>>>
>>
>> If it didn't get an adequate response, it's probably a good idea to
>> rebase the patch against latest code, and resend to the list: people
may
>> have deleted the old message.
>>
>> - Cole
>>
>
> This is the second time I have been asked to rebase. I submitted the
same
> patch in February. Daniel Veillard asked me to rebase that
patch. I
got
> very busy so I delayed submitting the patch until now. If
necessary
I'll
> submit again, but I'd like some assurance that I'm not
just wasting my
and
> your time.
>
If you rebase and resend the patch, I'll review it.
But seriously, everyone is always busy, and sometimes things slip
through the cracks. If in the future you want to call attention to a
forgotten patch, either rebase and resend with [RESEND] in the subject,
or reply to the original patch submission with a 'ping' or similar,
keeping the original mail intact so people don't have to check the
archives to find the mail you are talking about.
- Cole
7:42 a.m.
New subject: [libvirt] (Resend) Live Migration with non-shared storage for kvm
On 04/22/2010 11:06 AM, Kenneth Nagin wrote:
Support for live migration between hosts that do not share storage
was
added to qemu-kvm release 0.12.1.
It supports two flags:
-b migration without shared storage with full disk copy
-i migration without shared storage with incremental copy (same base image
shared between source and destination).
In order to support kvm's live migration with non-shared storage I added
two migration flags that user can invoke:
VIR_MIGRATE_NON_SHARED_DISK = (1 << 6), /* migration with non-shared
storage with full disk copy */
VIR_MIGRATE_NON_SHARED_INC = (1 << 7), /* migration with non-shared
storage with incremental copy */
/* (same base image shared
between source and destination) */
Likewise I add complementary flags to virsh migrate: non_shared_disk and
non_shared_inc
As Daniel B. suggested I also added internal flags to be passed in the
"background" parameter to the qemu monitor:
typedef enum {
QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0,
QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with
non-shared storage with full disk copy */
QEMU_MONITOR_MIGRATE_NON_SHARED_INC = 1 << 2, /* migration with
non-shared storage with incremental copy */
QEMU_MONITOR_MIGRATION_FLAGS_LAST
};
I updated qemu_driver.c's doNativeMigrate and doTunnelMigrate to map the
external flags to the internal flags.
I updated qemu_monitor_text's qemuMonitorTextMigrate to map the internal
flags to the qemu monitor's command line flags.
Also, qemudDomainSave ends up calling qemuMonitorTextMigrate, but it didn't
support the external flags so I assumed that it was not relevant.
I tested the live migration without shared storage (both flags) for native
and p2p with and without tunnelling. I also verified that the fix doesn't
affect normal migration with shared storage.
Here is the diff patch file from the current git:
(See attached file: libvirt_migration_ns_100422.patch)
-- Kenneth Nagin
Of course, I tell you to resend then forget to review. Sorry :(
Applying the patch (to last weeks checkout), there are some compiler warnings:
make sure you configure with --enable-compiler-warnings=error. 'make
syntax-check' also fails, so please address these.
Also, changes are probably needed to qemu_monitor_json.c to enable this
feature for future qemu releases. Probably a simple change, but I don't know
what it should be. Maybe Danpb can chime in here.
Finding a way to post the patch in-line will also probably get better
attention: just pasting it into the mail client will probably mangle the
patch, I'd recommend git send-email.
For the virsh changes, I wouldn't use underscores in cli flag names. Maybe
change this to 'copy-storage' and 'increment-storage'. Not too fond of
the
latter, maybe others have ideas.
Other than that, the code generally looks like (except for the compiler and
syntax-check warnings).
- Cole
(Here's the patch inline for the benefit of other reviewers)
diff --git a/include/libvirt/libvirt.h.in
b/include/libvirt/libvirt.h.in
index 4addc62..28fabd9 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -411,6 +411,10 @@ typedef enum {
VIR_MIGRATE_PERSIST_DEST = (1 << 3), /* persist the VM on the destination
*/
VIR_MIGRATE_UNDEFINE_SOURCE = (1 << 4), /* undefine the VM on the source */
VIR_MIGRATE_PAUSED = (1 << 5), /* pause on remote side */
+ VIR_MIGRATE_NON_SHARED_DISK = (1 << 6), /* migration with non-shared storage
with full disk copy */
+ VIR_MIGRATE_NON_SHARED_INC = (1 << 7), /* migration with non-shared storage
with incremental copy */
+ /* (same base image shared between source
and destination) */
+
} virDomainMigrateFlags;
/* Domain migration. */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5f4adfd..521638c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4871,7 +4871,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
if (header.compressed == QEMUD_SAVE_FORMAT_RAW) {
const char *args[] = { "cat", NULL };
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- rc = qemuMonitorMigrateToCommand(priv->mon, 1, args, path);
+ rc = qemuMonitorMigrateToCommand(priv->mon, QEMU_MONITOR_MIGRATE_BACKGROUND,
args, path);
qemuDomainObjExitMonitorWithDriver(driver, vm);
} else {
const char *prog = qemudSaveCompressionTypeToString(header.compressed);
@@ -4881,7 +4881,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
NULL
};
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- rc = qemuMonitorMigrateToCommand(priv->mon, 1, args, path);
+ rc = qemuMonitorMigrateToCommand(priv->mon, QEMU_MONITOR_MIGRATE_BACKGROUND,
args, path);
qemuDomainObjExitMonitorWithDriver(driver, vm);
}
@@ -5173,7 +5173,7 @@ static int qemudDomainCoreDump(virDomainPtr dom,
}
qemuDomainObjEnterMonitor(vm);
- ret = qemuMonitorMigrateToCommand(priv->mon, 1, args, path);
+ ret = qemuMonitorMigrateToCommand(priv->mon, QEMU_MONITOR_MIGRATE_BACKGROUND,
args, path);
qemuDomainObjExitMonitor(vm);
if (ret < 0)
@@ -9650,13 +9650,14 @@ cleanup:
static int doNativeMigrate(struct qemud_driver *driver,
virDomainObjPtr vm,
const char *uri,
- unsigned long flags ATTRIBUTE_UNUSED,
+ unsigned long flags ,
const char *dname ATTRIBUTE_UNUSED,
unsigned long resource)
{
int ret = -1;
xmlURIPtr uribits = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
+ int background_flags = 0;
/* Issue the migrate command. */
if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://"))
{
@@ -9684,7 +9685,13 @@ static int doNativeMigrate(struct qemud_driver *driver,
goto cleanup;
}
- if (qemuMonitorMigrateToHost(priv->mon, 1, uribits->server, uribits->port)
< 0) {
+ if (flags & VIR_MIGRATE_NON_SHARED_DISK)
+ background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK;
+
+ if (flags & VIR_MIGRATE_NON_SHARED_INC)
+ background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
+
+ if (qemuMonitorMigrateToHost(priv->mon, background_flags, uribits->server,
uribits->port) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
goto cleanup;
}
@@ -9769,7 +9776,7 @@ static int doTunnelMigrate(virDomainPtr dom,
unsigned long long qemuCmdFlags;
int status;
unsigned long long transferred, remaining, total;
-
+ int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
/*
* The order of operations is important here to avoid touching
* the source VM until we are very sure we can successfully
@@ -9854,11 +9861,16 @@ static int doTunnelMigrate(virDomainPtr dom,
/* 3. start migration on source */
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX)
- internalret = qemuMonitorMigrateToUnix(priv->mon, 1, unixfile);
+ if (flags & VIR_MIGRATE_NON_SHARED_DISK)
+ background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK;
+ if (flags & VIR_MIGRATE_NON_SHARED_INC)
+ background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX){
+ internalret = qemuMonitorMigrateToUnix(priv->mon, background_flags,
unixfile);
+ }
else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
const char *args[] = { "nc", "-U", unixfile, NULL };
- internalret = qemuMonitorMigrateToCommand(priv->mon, 1, args,
"/dev/null");
+ internalret = qemuMonitorMigrateToCommand(priv->mon, background_flags, args,
"/dev/null");
} else {
internalret = -1;
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 251233a..40a21f5 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -239,6 +239,13 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon,
unsigned long long *remaining,
unsigned long long *total);
+typedef enum {
+ QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0,
+ QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with non-shared
storage with full disk copy */
+ QEMU_MONITOR_MIGRATE_NON_SHARED_INC = 1 << 2, /* migration with non-shared
storage with incremental copy */
+ QEMU_MONITOR_MIGRATION_FLAGS_LAST
+};
+
int qemuMonitorMigrateToHost(qemuMonitorPtr mon,
int background,
const char *hostname,
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 6ad07b1..136ac12 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1132,18 +1132,19 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon,
char *info = NULL;
int ret = -1;
char *safedest = qemuMonitorEscapeArg(dest);
- const char *extra;
+ //const char *extra;
+ const char extra[25] = " ";
if (!safedest) {
virReportOOMError();
return -1;
}
-
- if (background)
- extra = "-d ";
- else
- extra = " ";
-
+ if (background & QEMU_MONITOR_MIGRATE_BACKGROUND)
+ strcat(extra," -d");
+ if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_DISK)
+ strcat(extra," -b");
+ if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_INC)
+ strcat(extra," -i");
if (virAsprintf(&cmd, "migrate %s\"%s\"", extra, safedest)
< 0) {
virReportOOMError();
goto cleanup;
diff --git a/tools/virsh.c b/tools/virsh.c
index b2a1538..bd8719e 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2804,6 +2804,8 @@ static const vshCmdOptDef opts_migrate[] = {
{"persistent", VSH_OT_BOOL, 0, N_("persist VM on
destination")},
{"undefinesource", VSH_OT_BOOL, 0, N_("undefine VM on
source")},
{"suspend", VSH_OT_BOOL, 0, N_("do not restart the domain on the
destination host")},
+ {"non_shared_disk", VSH_OT_BOOL, 0, N_("migration with non-shared
storage with full disk copy")},
+ {"non_shared_inc", VSH_OT_BOOL, 0, N_("migration with non-shared
storage with incremental copy (same base image shared between source and
destination)")},
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
{"desturi", VSH_OT_DATA, VSH_OFLAG_REQ, N_("connection URI of the
destination host")},
{"migrateuri", VSH_OT_DATA, 0, N_("migration URI, usually can be
omitted")},
@@ -2851,6 +2853,12 @@ cmdMigrate (vshControl *ctl, const vshCmd
*cmd)
if (vshCommandOptBool (cmd, "suspend"))
flags |= VIR_MIGRATE_PAUSED;
+ if (vshCommandOptBool (cmd, "non_shared_disk"))
+ flags |= VIR_MIGRATE_NON_SHARED_DISK;
+
+ if (vshCommandOptBool (cmd, "non_shared_inc"))
+ flags |= VIR_MIGRATE_NON_SHARED_INC;
+
if ((flags & VIR_MIGRATE_PEER2PEER) ||
vshCommandOptBool (cmd, "direct")) {
/* For peer2peer migration or direct migration we only expect one URI
10:26 a.m.
New subject: [libvirt] (Resend) Live Migration with non-shared storage for kvm
On 04/30/2010 06:42 AM, Cole Robinson wrote:
Other than that, the code generally looks like (except for the
compiler and
syntax-check warnings).
- Cole
(Here's the patch inline for the benefit of other reviewers)
In addition to Cole's comments, here's some style nits that 'make
syntax-check' won't pick up on...
> +++ b/src/qemu/qemu_driver.c
> @@ -4871,7 +4871,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char
*path,
> if (header.compressed == QEMUD_SAVE_FORMAT_RAW) {
> const char *args[] = { "cat", NULL };
> qemuDomainObjEnterMonitorWithDriver(driver, vm);
> - rc = qemuMonitorMigrateToCommand(priv->mon, 1, args, path);
> + rc = qemuMonitorMigrateToCommand(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path);
Where possible, wrap lines to fit 80 columns. For example,
rc = qemuMonitorMigrateToCommand(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND,
args, path);
> - unsigned long flags
ATTRIBUTE_UNUSED,
> + unsigned long flags ,
s/flags ,/flags,/
> const char *dname ATTRIBUTE_UNUSED,
> unsigned long resource)
> {
> int ret = -1;
> xmlURIPtr uribits = NULL;
> qemuDomainObjPrivatePtr priv = vm->privateData;
> + int background_flags = 0;
Flags should generally be 'unsigned int', to make it less likely to
cause problems with inadvertent sign extension if we ever reach 32 flags.
>
> /* Issue the migrate command. */
> if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri,
"tcp://")) {
> @@ -9684,7 +9685,13 @@ static int doNativeMigrate(struct qemud_driver *driver,
> goto cleanup;
> }
>
> - if (qemuMonitorMigrateToHost(priv->mon, 1, uribits->server,
uribits->port) < 0) {
> + if (flags & VIR_MIGRATE_NON_SHARED_DISK)
> + background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK;
That TAB will be caught by 'make syntax-check'.
> +++ b/src/qemu/qemu_monitor_text.c
> @@ -1132,18 +1132,19 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon,
> char *info = NULL;
> int ret = -1;
> char *safedest = qemuMonitorEscapeArg(dest);
> - const char *extra;
> + //const char *extra;
No point leaving a dead comment; version-control is there to let us see
what extra was declared as before your patch changed it to:
> + const char extra[25] = " ";
Why 25? That seems like a magic number, and it wastes space compared to
the maximum amount that you strcat() into it below.
>
> if (!safedest) {
> virReportOOMError();
> return -1;
> }
> -
> - if (background)
> - extra = "-d ";
> - else
> - extra = " ";
> -
> + if (background & QEMU_MONITOR_MIGRATE_BACKGROUND)
> + strcat(extra," -d");
> + if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_DISK)
> + strcat(extra," -b");
> + if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_INC)
> + strcat(extra," -i");
> if (virAsprintf(&cmd, "migrate %s\"%s\"", extra,
safedest) < 0) {
That combination of strcat() and virAsprintf() looks bad. In general,
you should avoid strcat() (it often has overflow problems). And since
there is already allocation going on with virAsprintf, it seems like the
better way to write this would be to convert both strcat and virAsprintf
into multiple uses of the virBuffer* API, in order to build a single
malloc'd string incrementally.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:13 a.m.
New subject: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
Eric Blake <eblake(a)redhat.com> wrote on 30/04/2010 18:26:04:
From: Eric Blake <eblake(a)redhat.com>
To: Cole Robinson <crobinso(a)redhat.com>
Cc: Kenneth Nagin/Haifa/IBM@IBMIL, list libvirt <libvir-list(a)redhat.com>
Date: 30/04/2010 18:26
Subject: Re: [libvirt] (Resend) Live Migration with non-shared storage
for kvm
On 04/30/2010 06:42 AM, Cole Robinson wrote:
> Other than that, the code generally looks like (except for the compiler
and
> syntax-check warnings).
>
> - Cole
>
> (Here's the patch inline for the benefit of other reviewers)
In addition to Cole's comments, here's some style nits that 'make
syntax-check' won't pick up on...
>> +++ b/src/qemu/qemu_driver.c
>> @@ -4871,7 +4871,7 @@ static int qemudDomainSaveFlag(virDomainPtr
dom, const char *path,
>> if (header.compressed == QEMUD_SAVE_FORMAT_RAW) {
>> const char *args[] = { "cat", NULL };
>> qemuDomainObjEnterMonitorWithDriver(driver, vm);
>> - rc = qemuMonitorMigrateToCommand(priv->mon, 1, args, path);
>> + rc = qemuMonitorMigrateToCommand(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path);
Where possible, wrap lines to fit 80 columns. For example,
rc = qemuMonitorMigrateToCommand(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND,
args, path);
>> - unsigned long flags ATTRIBUTE_UNUSED,
>> + unsigned long flags ,
s/flags ,/flags,/
>> const char *dname ATTRIBUTE_UNUSED,
>> unsigned long resource)
>> {
>> int ret = -1;
>> xmlURIPtr uribits = NULL;
>> qemuDomainObjPrivatePtr priv = vm->privateData;
>> + int background_flags = 0;
Flags should generally be 'unsigned int', to make it less likely to
cause problems with inadvertent sign extension if we ever reach 32 flags.
>>
>> /* Issue the migrate command. */
>> if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri,
"tcp://")) {
>> @@ -9684,7 +9685,13 @@ static int doNativeMigrate(struct
qemud_driver *driver,
>> goto cleanup;
>> }
>>
>> - if (qemuMonitorMigrateToHost(priv->mon, 1, uribits->server,
uribits->port) < 0) {
>> + if (flags & VIR_MIGRATE_NON_SHARED_DISK)
>> + background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK;
That TAB will be caught by 'make syntax-check'.
>> +++ b/src/qemu/qemu_monitor_text.c
>> @@ -1132,18 +1132,19 @@ static int qemuMonitorTextMigrate
(qemuMonitorPtr mon,
>> char *info = NULL;
>> int ret = -1;
>> char *safedest = qemuMonitorEscapeArg(dest);
>> - const char *extra;
>> + //const char *extra;
No point leaving a dead comment; version-control is there to let us see
what extra was declared as before your patch changed it to:
>> + const char extra[25] = " ";
Why 25? That seems like a magic number, and it wastes space compared to
the maximum amount that you strcat() into it below.
>>
>> if (!safedest) {
>> virReportOOMError();
>> return -1;
>> }
>> -
>> - if (background)
>> - extra = "-d ";
>> - else
>> - extra = " ";
>> -
>> + if (background & QEMU_MONITOR_MIGRATE_BACKGROUND)
>> + strcat(extra," -d");
>> + if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_DISK)
>> + strcat(extra," -b");
>> + if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_INC)
>> + strcat(extra," -i");
>> if (virAsprintf(&cmd, "migrate %s\"%s\"", extra,
safedest) < 0) {
That combination of strcat() and virAsprintf() looks bad. In general,
you should avoid strcat() (it often has overflow problems). And since
there is already allocation going on with virAsprintf, it seems like the
better way to write this would be to convert both strcat and virAsprintf
into multiple uses of the virBuffer* API, in order to build a single
malloc'd string incrementally.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
[attachment "signature.asc" deleted by Kenneth Nagin/Haifa/IBM]
I
implemented the suggested changes by Cole and Eric, namely:
replaced tabs with blanks (Cole)
replace virsh flag names with copy-storage-all and copy-storage-inc
(Cole)
replaced strcat with virBuffer* API (Eric)
replaced char extra[25] with virBuffer declaration (Eric)
replace int background with unsigned int background (Eric)
I did not make changes to JSON (suggested by Cole) because I didn't know
how to verify the fixes.
I did not replace flags with unsigned int in qemuMonitorMigrateToCommand
(suggested by Eric) because these flags are part of the external interface
and past with rpcgen. I worried that the change might create more
problems, and not worth the small benefit in style.
This the fix:
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index f296d16..db107cc 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -411,6 +411,10 @@ typedef enum {
VIR_MIGRATE_PERSIST_DEST = (1 << 3), /* persist the VM on the
destination */
VIR_MIGRATE_UNDEFINE_SOURCE = (1 << 4), /* undefine the VM on the
source */
VIR_MIGRATE_PAUSED = (1 << 5), /* pause on remote side */
+ VIR_MIGRATE_NON_SHARED_DISK = (1 << 6), /* migration with non-shared
storage with full disk copy */
+ VIR_MIGRATE_NON_SHARED_INC = (1 << 7), /* migration with non-shared
storage with incremental copy */
+ /* (same base image shared
between source and destination) */
+
} virDomainMigrateFlags;
/* Domain migration. */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 08cff00..9a3461d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4958,7 +4958,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
if (header.compressed == QEMUD_SAVE_FORMAT_RAW) {
const char *args[] = { "cat", NULL };
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- rc = qemuMonitorMigrateToFile(priv->mon, 1, args, path, offset);
+ rc = qemuMonitorMigrateToFile(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
qemuDomainObjExitMonitorWithDriver(driver, vm);
} else {
const char *prog = qemudSaveCompressionTypeToString
(header.compressed);
@@ -4968,7 +4968,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
NULL
};
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- rc = qemuMonitorMigrateToFile(priv->mon, 1, args, path, offset);
+ rc = qemuMonitorMigrateToFile(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
qemuDomainObjExitMonitorWithDriver(driver, vm);
}
@@ -5286,9 +5286,8 @@ static int qemudDomainCoreDump(virDomainPtr dom,
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- ret = qemuMonitorMigrateToFile(priv->mon, 1, args, path, 0);
+ ret = qemuMonitorMigrateToFile(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, 0);
qemuDomainObjExitMonitorWithDriver(driver, vm);
-
if (ret < 0)
goto endjob;
@@ -9885,13 +9884,14 @@ cleanup:
static int doNativeMigrate(struct qemud_driver *driver,
virDomainObjPtr vm,
const char *uri,
- unsigned long flags ATTRIBUTE_UNUSED,
+ unsigned long flags ,
const char *dname ATTRIBUTE_UNUSED,
unsigned long resource)
{
int ret = -1;
xmlURIPtr uribits = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
+ unsigned int background_flags = 0;
/* Issue the migrate command. */
if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://"))
{
@@ -9919,7 +9919,13 @@ static int doNativeMigrate(struct qemud_driver
*driver,
goto cleanup;
}
- if (qemuMonitorMigrateToHost(priv->mon, 1, uribits->server, uribits->
port) < 0) {
+ if (flags & VIR_MIGRATE_NON_SHARED_DISK)
+ background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK;
+
+ if (flags & VIR_MIGRATE_NON_SHARED_INC)
+ background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
+
+ if (qemuMonitorMigrateToHost(priv->mon, background_flags, uribits->
server, uribits->port) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
goto cleanup;
}
@@ -10004,7 +10010,7 @@ static int doTunnelMigrate(virDomainPtr dom,
unsigned long long qemuCmdFlags;
int status;
unsigned long long transferred, remaining, total;
-
+ unsigned int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
/*
* The order of operations is important here to avoid touching
* the source VM until we are very sure we can successfully
@@ -10089,11 +10095,16 @@ static int doTunnelMigrate(virDomainPtr dom,
/* 3. start migration on source */
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX)
- internalret = qemuMonitorMigrateToUnix(priv->mon, 1, unixfile);
+ if (flags & VIR_MIGRATE_NON_SHARED_DISK)
+ background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK;
+ if (flags & VIR_MIGRATE_NON_SHARED_INC)
+ background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX){
+ internalret = qemuMonitorMigrateToUnix(priv->mon,
background_flags, unixfile);
+ }
else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
const char *args[] = { "nc", "-U", unixfile, NULL };
- internalret = qemuMonitorMigrateToCommand(priv->mon, 1, args);
+ internalret = qemuMonitorMigrateToCommand(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args);
} else {
internalret = -1;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fca8590..443d216 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1161,7 +1161,7 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon,
int qemuMonitorMigrateToHost(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char *hostname,
int port)
{
@@ -1178,7 +1178,7 @@ int qemuMonitorMigrateToHost(qemuMonitorPtr mon,
int qemuMonitorMigrateToCommand(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char * const *argv)
{
int ret;
@@ -1193,7 +1193,7 @@ int qemuMonitorMigrateToCommand(qemuMonitorPtr mon,
}
int qemuMonitorMigrateToFile(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char * const *argv,
const char *target,
unsigned long long offset)
@@ -1217,7 +1217,7 @@ int qemuMonitorMigrateToFile(qemuMonitorPtr mon,
}
int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char *unixfile)
{
int ret;
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 3fa83b7..9760b4c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -241,25 +241,32 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon,
unsigned long long *remaining,
unsigned long long *total);
+typedef enum {
+ QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0,
+ QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with
non-shared storage with full disk copy */
+ QEMU_MONITOR_MIGRATE_NON_SHARED_INC = 1 << 2, /* migration with
non-shared storage with incremental copy */
+ QEMU_MONITOR_MIGRATION_FLAGS_LAST
+} QEMU_MONITOR_MIGRATE;
+
int qemuMonitorMigrateToHost(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char *hostname,
int port);
int qemuMonitorMigrateToCommand(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char * const *argv);
# define QEMU_MONITOR_MIGRATE_TO_FILE_BS 512llu
int qemuMonitorMigrateToFile(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char * const *argv,
const char *target,
unsigned long long offset);
int qemuMonitorMigrateToUnix(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char *unixfile);
int qemuMonitorMigrateCancel(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 3f917bf..ff79663 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -38,6 +38,7 @@
#include "driver.h"
#include "datatypes.h"
#include "virterror_internal.h"
+#include "buf.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -1125,26 +1126,32 @@ cleanup:
static int qemuMonitorTextMigrate(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char *dest)
{
char *cmd = NULL;
char *info = NULL;
int ret = -1;
char *safedest = qemuMonitorEscapeArg(dest);
- const char *extra;
+ virBuffer extra = VIR_BUFFER_INITIALIZER;
if (!safedest) {
virReportOOMError();
return -1;
}
-
- if (background)
- extra = "-d ";
- else
- extra = " ";
-
- if (virAsprintf(&cmd, "migrate %s\"%s\"", extra, safedest)
< 0) {
+ virBufferAddLit(&extra, " ");
+ if (background & QEMU_MONITOR_MIGRATE_BACKGROUND)
+ virBufferAddLit(&extra, " -d");
+ if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_DISK)
+ virBufferAddLit(&extra, " -b");
+ if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_INC)
+ virBufferAddLit(&extra, " -i");
+ if (virBufferError(&extra)) {
+ virBufferFreeAndReset(&extra);
+ virReportOOMError();
+ return -1;
+ }
+ if (virAsprintf(&cmd, "migrate %s\"%s\"",
virBufferContentAndReset
(&extra), safedest) < 0) {
virReportOOMError();
goto cleanup;
}
@@ -1180,7 +1187,7 @@ cleanup:
}
int qemuMonitorTextMigrateToHost(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char *hostname,
int port)
{
@@ -1201,7 +1208,7 @@ int qemuMonitorTextMigrateToHost(qemuMonitorPtr mon,
int qemuMonitorTextMigrateToCommand(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char * const *argv)
{
char *argstr;
@@ -1228,7 +1235,7 @@ cleanup:
}
int qemuMonitorTextMigrateToFile(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char * const *argv,
const char *target,
unsigned long long offset)
@@ -1269,7 +1276,7 @@ cleanup:
}
int qemuMonitorTextMigrateToUnix(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char *unixfile)
{
char *dest = NULL;
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index 23c3a45..7d1bc56 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -93,22 +93,22 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr
mon,
unsigned long long *total);
int qemuMonitorTextMigrateToHost(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char *hostname,
int port);
int qemuMonitorTextMigrateToCommand(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char * const *argv);
int qemuMonitorTextMigrateToFile(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char * const *argv,
const char *target,
unsigned long long offset);
int qemuMonitorTextMigrateToUnix(qemuMonitorPtr mon,
- int background,
+ unsigned int background,
const char *unixfile);
int qemuMonitorTextMigrateCancel(qemuMonitorPtr mon);
diff --git a/tools/virsh.c b/tools/virsh.c
index eb11a78..f96d31a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2851,6 +2851,8 @@ static const vshCmdOptDef opts_migrate[] = {
{"persistent", VSH_OT_BOOL, 0, N_("persist VM on destination")},
{"undefinesource", VSH_OT_BOOL, 0, N_("undefine VM on source")},
{"suspend", VSH_OT_BOOL, 0, N_("do not restart the domain on the
destination host")},
+ {"copy-storage-all", VSH_OT_BOOL, 0, N_("migration with non-shared
storage with full disk copy")},
+ {"copy-storage-inc", VSH_OT_BOOL, 0, N_("migration with non-shared
storage with incremental copy (same base image shared between source and
destination)")},
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
{"desturi", VSH_OT_DATA, VSH_OFLAG_REQ, N_("connection URI of the
destination host")},
{"migrateuri", VSH_OT_DATA, 0, N_("migration URI, usually can be
omitted")},
@@ -2898,6 +2900,12 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool (cmd, "suspend"))
flags |= VIR_MIGRATE_PAUSED;
+ if (vshCommandOptBool (cmd, "copy-storage-all"))
+ flags |= VIR_MIGRATE_NON_SHARED_DISK;
+
+ if (vshCommandOptBool (cmd, "copy-storage-inc"))
+ flags |= VIR_MIGRATE_NON_SHARED_INC;
+
if ((flags & VIR_MIGRATE_PEER2PEER) ||
vshCommandOptBool (cmd, "direct")) {
/* For peer2peer migration or direct migration we only expect one
URI
This the patch file:
(See attached file: libvirt_migration_ns_100503.patch)
regards,
Kenneth Nagin
5:04 p.m.
New subject: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
On 05/03/2010 07:52 AM, Cole Robinson wrote:
On 05/03/2010 04:13 AM, Kenneth Nagin wrote:
>
...
> This the patch file:
> (See attached file: libvirt_migration_ns_100503.patch)
>
ACK, patch looks fine now.
Kenneth,
I added a commit message, broke a few lines to fit in 80 columns, added
a virCheckFlags to doNativeMigrate (which required a type change), and
removed an extra space in the generated line:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 5823e10..47ae52c 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -4958,7 +4958,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
if (header.compressed == QEMUD_SAVE_FORMAT_RAW) {
const char *args[] = { "cat", NULL };
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- rc = qemuMonitorMigrateToFile(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
+ rc = qemuMonitorMigrateToFile(priv->mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, offset);
qemuDomainObjExitMonitorWithDriver(driver, vm);
} else {
const char *prog =
qemudSaveCompressionTypeToString(header.compressed);
@@ -4968,7 +4970,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
NULL
};
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- rc = qemuMonitorMigrateToFile(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
+ rc = qemuMonitorMigrateToFile(priv->mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, offset);
qemuDomainObjExitMonitorWithDriver(driver, vm);
}
@@ -5286,7 +5290,9 @@ static int qemudDomainCoreDump(virDomainPtr dom,
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- ret = qemuMonitorMigrateToFile(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, 0);
+ ret = qemuMonitorMigrateToFile(priv->mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, 0);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret < 0)
goto endjob;
@@ -9884,7 +9890,7 @@ cleanup:
static int doNativeMigrate(struct qemud_driver *driver,
virDomainObjPtr vm,
const char *uri,
- unsigned long flags ,
+ unsigned int flags,
const char *dname ATTRIBUTE_UNUSED,
unsigned long resource)
{
@@ -9893,6 +9899,9 @@ static int doNativeMigrate(struct qemud_driver
*driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
unsigned int background_flags = 0;
+ virCheckFlags(VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC,
+ -1);
+
/* Issue the migrate command. */
if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://"))
{
/* HACK: source host generates bogus URIs, so fix them up */
@@ -9925,7 +9934,8 @@ static int doNativeMigrate(struct qemud_driver
*driver,
if (flags & VIR_MIGRATE_NON_SHARED_INC)
background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
- if (qemuMonitorMigrateToHost(priv->mon, background_flags,
uribits->server, uribits->port) < 0) {
+ if (qemuMonitorMigrateToHost(priv->mon, background_flags,
uribits->server,
+ uribits->port) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
goto cleanup;
}
@@ -10011,6 +10021,7 @@ static int doTunnelMigrate(virDomainPtr dom,
int status;
unsigned long long transferred, remaining, total;
unsigned int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
+
/*
* The order of operations is important here to avoid touching
* the source VM until we are very sure we can successfully
@@ -10100,7 +10111,8 @@ static int doTunnelMigrate(virDomainPtr dom,
if (flags & VIR_MIGRATE_NON_SHARED_INC)
background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX){
- internalret = qemuMonitorMigrateToUnix(priv->mon,
background_flags, unixfile);
+ internalret = qemuMonitorMigrateToUnix(priv->mon, background_flags,
+ unixfile);
}
else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
const char *args[] = { "nc", "-U", unixfile, NULL };
diff --git i/src/qemu/qemu_monitor_text.c w/src/qemu/qemu_monitor_text.c
index ff79663..3119600 100644
--- i/src/qemu/qemu_monitor_text.c
+++ w/src/qemu/qemu_monitor_text.c
@@ -1139,7 +1139,7 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon,
virReportOOMError();
return -1;
}
- virBufferAddLit(&extra, " ");
+
if (background & QEMU_MONITOR_MIGRATE_BACKGROUND)
virBufferAddLit(&extra, " -d");
if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_DISK)
Then I verified that it passes 'make check' and 'make syntax-check', and
finally pushed this in your name. Thanks again for the contribution,
and for the reminders to review it.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:26 a.m.
New subject: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
Ph: +972-4-8296227
Cell: 054-6976227
Fx: +972-4- 8296113
"Do not look back in anger, or forward in fear, but around in awareness."
--James Thurber
Eric Blake <eblake(a)redhat.com> wrote on 05/05/2010 01:04:23:
From: Eric Blake <eblake(a)redhat.com>
To: Cole Robinson <crobinso(a)redhat.com>
Cc: Kenneth Nagin/Haifa/IBM@IBMIL, list libvirt <libvir-list(a)redhat.com>
Date: 05/05/2010 01:04
Subject: Re: [libvirt] (Resend with updates) Live Migration with
non-shared storage for kvm
On 05/03/2010 07:52 AM, Cole Robinson wrote:
> On 05/03/2010 04:13 AM, Kenneth Nagin wrote:
>>
>
> ...
>
>> This the patch file:
>> (See attached file: libvirt_migration_ns_100503.patch)
>>
>
> ACK, patch looks fine now.
Kenneth,
I added a commit message, broke a few lines to fit in 80 columns, added
a virCheckFlags to doNativeMigrate (which required a type change), and
removed an extra space in the generated line:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 5823e10..47ae52c 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -4958,7 +4958,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
if (header.compressed == QEMUD_SAVE_FORMAT_RAW) {
const char *args[] = { "cat", NULL };
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- rc = qemuMonitorMigrateToFile(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
+ rc = qemuMonitorMigrateToFile(priv->mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, offset);
qemuDomainObjExitMonitorWithDriver(driver, vm);
} else {
const char *prog =
qemudSaveCompressionTypeToString(header.compressed);
@@ -4968,7 +4970,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
NULL
};
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- rc = qemuMonitorMigrateToFile(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
+ rc = qemuMonitorMigrateToFile(priv->mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, offset);
qemuDomainObjExitMonitorWithDriver(driver, vm);
}
@@ -5286,7 +5290,9 @@ static int qemudDomainCoreDump(virDomainPtr dom,
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- ret = qemuMonitorMigrateToFile(priv->mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, 0);
+ ret = qemuMonitorMigrateToFile(priv->mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, 0);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret < 0)
goto endjob;
@@ -9884,7 +9890,7 @@ cleanup:
static int doNativeMigrate(struct qemud_driver *driver,
virDomainObjPtr vm,
const char *uri,
- unsigned long flags ,
+ unsigned int flags,
const char *dname ATTRIBUTE_UNUSED,
unsigned long resource)
{
@@ -9893,6 +9899,9 @@ static int doNativeMigrate(struct qemud_driver
*driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
unsigned int background_flags = 0;
+ virCheckFlags(VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC,
+ -1);
+
/* Issue the migrate command. */
if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://"))
{
/* HACK: source host generates bogus URIs, so fix them up */
@@ -9925,7 +9934,8 @@ static int doNativeMigrate(struct qemud_driver
*driver,
if (flags & VIR_MIGRATE_NON_SHARED_INC)
background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
- if (qemuMonitorMigrateToHost(priv->mon, background_flags,
uribits->server, uribits->port) < 0) {
+ if (qemuMonitorMigrateToHost(priv->mon, background_flags,
uribits->server,
+ uribits->port) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
goto cleanup;
}
@@ -10011,6 +10021,7 @@ static int doTunnelMigrate(virDomainPtr dom,
int status;
unsigned long long transferred, remaining, total;
unsigned int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
+
/*
* The order of operations is important here to avoid touching
* the source VM until we are very sure we can successfully
@@ -10100,7 +10111,8 @@ static int doTunnelMigrate(virDomainPtr dom,
if (flags & VIR_MIGRATE_NON_SHARED_INC)
background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX){
- internalret = qemuMonitorMigrateToUnix(priv->mon,
background_flags, unixfile);
+ internalret = qemuMonitorMigrateToUnix(priv->mon,
background_flags,
+ unixfile);
}
else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
const char *args[] = { "nc", "-U", unixfile, NULL };
diff --git i/src/qemu/qemu_monitor_text.c w/src/qemu/qemu_monitor_text.c
index ff79663..3119600 100644
--- i/src/qemu/qemu_monitor_text.c
+++ w/src/qemu/qemu_monitor_text.c
@@ -1139,7 +1139,7 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr
mon,
virReportOOMError();
return -1;
}
- virBufferAddLit(&extra, " ");
+
if (background & QEMU_MONITOR_MIGRATE_BACKGROUND)
virBufferAddLit(&extra, " -d");
if (background & QEMU_MONITOR_MIGRATE_NON_SHARED_DISK)
Then I verified that it passes 'make check' and 'make syntax-check', and
finally pushed this in your name. Thanks again for the contribution,
and for the reminders to review it.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
[attachment "signature.asc" deleted by Kenneth Nagin/Haifa/IBM]
Thanks both of you for your assistance and thorough reviews.
Since this is my first contribution, what happens next? How do I know
whether it was accepted into an official release?
regards,
Kenneth Nagin
10:24 a.m.
New subject: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
On 05/05/2010 12:26 AM, Kenneth Nagin wrote:
Eric Blake <eblake(a)redhat.com> wrote on 05/05/2010 01:04:23:
[feel free to trim content unrelated to your reply, it makes it easier
to get to your comments]
> Then I verified that it passes 'make check' and 'make
syntax-check', and
> finally pushed this in your name. Thanks again for the contribution,
> and for the reminders to review it.
>
> --
> Eric Blake eblake(a)redhat.com +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
>
> [attachment "signature.asc" deleted by Kenneth Nagin/Haifa/IBM]
Thanks both of you for your assistance and thorough reviews.
Since this is my first contribution, what happens next? How do I know
whether it was accepted into an official release?
It's already been applied in the upstream git repository. Which means
that the next libvirt release (probably 0.8.2, and hopefully at the end
of May per our typical release schedule) will include your changes.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:40 a.m.
New subject: [libvirt] libvirt 0.8.2?
When is libvirt 0.8.2 expected to be released?
Kenneth Nagin
From: Eric Blake <eblake(a)redhat.com>
To: Kenneth Nagin/Haifa/IBM@IBMIL
Cc: list libvirt <libvir-list(a)redhat.com>
Date: 05/05/2010 18:24
Subject: Re: [libvirt] (Resend with updates) Live Migration with
non-shared storage for kvm
On 05/05/2010 12:26 AM, Kenneth Nagin wrote:
Eric Blake <eblake(a)redhat.com> wrote on 05/05/2010 01:04:23:
[feel free to trim content unrelated to your reply, it makes it easier
to get to your comments]
> Then I verified that it passes 'make check' and 'make
syntax-check', and
> finally pushed this in your name. Thanks again for the contribution,
> and for the reminders to review it.
>
> --
> Eric Blake eblake(a)redhat.com +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
>
> [attachment "signature.asc" deleted by Kenneth Nagin/Haifa/IBM]
Thanks both of you for your assistance and thorough reviews.
Since this is my first contribution, what happens next? How do I know
whether it was accepted into an official release?
It's already been applied in the upstream git repository. Which means
that the next libvirt release (probably 0.8.2, and hopefully at the end
of May per our typical release schedule) will include your changes.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
[attachment "signature.asc" deleted by Kenneth Nagin/Haifa/IBM]
3:16 a.m.
New subject: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
On Mon, May 03, 2010 at 09:52:10AM -0400, Cole Robinson wrote:
On 05/03/2010 04:13 AM, Kenneth Nagin wrote:
>
...
> This the patch file:
> (See attached file: libvirt_migration_ns_100503.patch)
>
ACK, patch looks fine now.
No objections to the patch that was committed, but FYI there is an
improvement that can be done to it.
If you run 'virsh domjobinfo $GUEST' while migration is running it
tells you the progress of migration. It gives overall progress, and
memory progress. The APi was designed to allow us to report storage
progress too. To wire this up look at the method
qemuDomainWaitForMigrationComplete
where it updates the priv->jobInfo variables. You'll also need to add
extra params to the qemuMonitorGetMigrationStatus() method to return
the disk processed/remaining/total stats
virsh should already be able to report this data once the QEMU driver
is wired up
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
1:23 a.m.
New subject: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm
"Daniel P. Berrange" <berrange(a)redhat.com> wrote on
05/05/2010 11:16:37:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
To: Cole Robinson <crobinso(a)redhat.com>
Cc: Kenneth Nagin/Haifa/IBM@IBMIL, list libvirt <libvir-list(a)redhat.com>
Date: 05/05/2010 11:16
Subject: Re: [libvirt] (Resend with updates) Live Migration with
non-shared storage for kvm
On Mon, May 03, 2010 at 09:52:10AM -0400, Cole Robinson wrote:
> On 05/03/2010 04:13 AM, Kenneth Nagin wrote:
> >
>
> ...
>
> > This the patch file:
> > (See attached file: libvirt_migration_ns_100503.patch)
> >
>
> ACK, patch looks fine now.
No objections to the patch that was committed, but FYI there is an
improvement that can be done to it.
If you run 'virsh domjobinfo $GUEST' while migration is running it
tells you the progress of migration. It gives overall progress, and
memory progress. The APi was designed to allow us to report storage
progress too. To wire this up look at the method
qemuDomainWaitForMigrationComplete
where it updates the priv->jobInfo variables. You'll also need to add
extra params to the qemuMonitorGetMigrationStatus() method to return
the disk processed/remaining/total stats
virsh should already be able to report this data once the QEMU driver
is wired up
Regards,
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/:|
|: http://libvirt.org -o- http://virt-manager.org -o-
http://deltacloud.org:|
|: http://autobuild.org -o-
http://search.cpan.org/~danberr/:|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742
7D3B
9505 :|
Thank you. I will look into implementing the improvement when I have time.
However, it won't be for a while since I have some pressing deadlines that
I have to meet in the next few months.
regards,
Kenneth Nagin
Ph: +972-4-8296227
Cell: 054-6976227
Fx: +972-4- 8296113
http://researcher.watson.ibm.com/researcher/view.php?person=il-NAGIN
http://www.reservoir-fp7.eu/
3:29 a.m.
New subject: [libvirt] make syntax-check: [sc_vulnerable_makefile_CVE-2009-4029] Error 1
Cole Robinson <crobinso(a)redhat.com> wrote on 30/04/2010
15:42:05:
From: Cole Robinson <crobinso(a)redhat.com>
To: Kenneth Nagin/Haifa/IBM@IBMIL
Cc: "Daniel P. Berrange" <berrange(a)redhat.com>, list libvirt
<libvir-list(a)redhat.com>, Daniel Veillard <veillard(a)redhat.com>
Date: 30/04/2010 15:42
Subject: Re: [libvirt] (Resend) Live Migration with non-shared storage
for kvm
Applying the patch (to last weeks checkout), there are some
compilerwarnings:
make sure you configure with --enable-compiler-warnings=error.
'make
syntax-check' also fails, so please address these.
But I get this error message when compiling with 'make syntax-check':
2.14 unmarked_diagnostics
vulnerable_makefile_CVE-2009-4029
./Makefile.in:1283: -find $(distdir) -type d ! -perm -777 -exec chmod a+rwx
{} \; -o \
maint.mk: the above files are vulnerable; beware of running
"make dist*" rules, and upgrade to fixed automake
see http://bugzilla.redhat.com/542609 for details
make: *** [sc_vulnerable_makefile_CVE-2009-4029] Error 1
This problem is unrelated to any changes that I made and appearantly the
compile completes because make install works properly.
Any suggestions on how to resolve this error message.
regards,
Kenneth Nagin
3:36 a.m.
New subject: [libvirt] make syntax-check: [sc_vulnerable_makefile_CVE-2009-4029] Error 1
Kenneth Nagin wrote:
>Cole Robinson <crobinso(a)redhat.com> wrote on 30/04/2010
15:42:05:
> From: Cole Robinson <crobinso(a)redhat.com>
> To: Kenneth Nagin/Haifa/IBM@IBMIL
> Cc: "Daniel P. Berrange" <berrange(a)redhat.com>, list libvirt
> <libvir-list(a)redhat.com>, Daniel Veillard <veillard(a)redhat.com>
> Date: 30/04/2010 15:42
> Subject: Re: [libvirt] (Resend) Live Migration with non-shared storage
for kvm
>
> Applying the patch (to last weeks checkout), there are some
compilerwarnings:
> make sure you configure with --enable-compiler-warnings=error. 'make
> syntax-check' also fails, so please address these.
But I get this error message when compiling with 'make syntax-check':
2.14 unmarked_diagnostics
vulnerable_makefile_CVE-2009-4029
./Makefile.in:1283: -find $(distdir) -type d ! -perm -777 -exec chmod a+rwx
{} \; -o \
maint.mk: the above files are vulnerable; beware of running
"make dist*" rules, and upgrade to fixed automake
see http://bugzilla.redhat.com/542609 for details
make: *** [sc_vulnerable_makefile_CVE-2009-4029] Error 1
This problem is unrelated to any changes that I made and appearantly the
compile completes because make install works properly.
Any suggestions on how to resolve this error message.
That means you are using a version of automake
that lacks the fix for the referenced bug. Upgrading
to a patched version of automake, and regenerating all
Makefile.in files will fix it.
If you run any make rule that runs that find command,
you may expose yourself to a nasty exploit.
6:35 a.m.
New subject: [libvirt] make syntax-check: [sc_vulnerable_makefile_CVE-2009-4029] Error 1
Jim Meyering <jim(a)meyering.net> wrote on 03/05/2010 11:36:59:
From: Jim Meyering <jim(a)meyering.net>
To: Kenneth Nagin/Haifa/IBM@IBMIL
Cc: Cole Robinson <crobinso(a)redhat.com>, list libvirt
<libvir-list(a)redhat.com>
Date: 03/05/2010 11:37
Subject: Re: [libvirt] make syntax-check:
[sc_vulnerable_makefile_CVE-2009-4029] Error 1
Kenneth Nagin wrote:
>>Cole Robinson <crobinso(a)redhat.com> wrote on 30/04/2010 15:42:05:
>
>> From: Cole Robinson <crobinso(a)redhat.com>
>> To: Kenneth Nagin/Haifa/IBM@IBMIL
>> Cc: "Daniel P. Berrange" <berrange(a)redhat.com>, list libvirt
>> <libvir-list(a)redhat.com>, Daniel Veillard <veillard(a)redhat.com>
>> Date: 30/04/2010 15:42
>> Subject: Re: [libvirt] (Resend) Live Migration with non-shared storage
> for kvm
>>
>> Applying the patch (to last weeks checkout), there are some
> compilerwarnings:
>> make sure you configure with --enable-compiler-warnings=error. 'make
>> syntax-check' also fails, so please address these.
>
> But I get this error message when compiling with 'make syntax-check':
>
> 2.14 unmarked_diagnostics
> vulnerable_makefile_CVE-2009-4029
> ./Makefile.in:1283: -find $(distdir) -type d ! -perm -777 -exec
chmod a+rwx
> {} \; -o \
> maint.mk: the above files are vulnerable; beware of running
> "make dist*" rules, and upgrade to fixed automake
> see http://bugzilla.redhat.com/542609 for details
> make: *** [sc_vulnerable_makefile_CVE-2009-4029] Error 1
>
> This problem is unrelated to any changes that I made and appearantly
the
> compile completes because make install works properly.
>
> Any suggestions on how to resolve this error message.
That means you are using a version of automake
that lacks the fix for the referenced bug. Upgrading
to a patched version of automake, and regenerating all
Makefile.in files will fix it.
If you run any make rule that runs that find command,
you may expose yourself to a nasty exploit.
I upgrade automake, but now make
syntac-check fails with because LIBTOOL is
undefined. I followed the instructions in the error message but it didn't
help.
[root@hoover libvirt]# make syntax-check
cd .
&& /bin/sh /gpfs/reservoir/nagin/workspace/libvirt_tip/libvirt/build-aux/missing
--run automake-1.10 --gnu
gnulib/lib/Makefile.am:21: Libtool library used but `LIBTOOL' is undefined
gnulib/lib/Makefile.am:21: The usual way to define `LIBTOOL' is to add
`AC_PROG_LIBTOOL'
gnulib/lib/Makefile.am:21: to `configure.ac' and run `aclocal' and
`autoconf' again.
gnulib/lib/Makefile.am:21: If `AC_PROG_LIBTOOL' is in `configure.ac',
make sure
gnulib/lib/Makefile.am:21: its definition is in aclocal's search path.
regards
Kenneth Nagin
6:45 a.m.
New subject: [libvirt] make syntax-check: [sc_vulnerable_makefile_CVE-2009-4029] Error 1
Kenneth Nagin wrote:
I upgrade automake, but now make syntac-check fails with because
LIBTOOL is
undefined. I followed the instructions in the error message but it didn't
help.
[root@hoover libvirt]# make syntax-check
cd .
&& /bin/sh /gpfs/reservoir/nagin/workspace/libvirt_tip/libvirt/build-aux/missing
--run automake-1.10 --gnu
gnulib/lib/Makefile.am:21: Libtool library used but `LIBTOOL' is undefined
gnulib/lib/Makefile.am:21: The usual way to define `LIBTOOL' is to add
`AC_PROG_LIBTOOL'
This is independent of "syntax-check".
I suspect you should run "make distclean" or use some
other method (e.g., like starting from a fresh clone) to ensure
that you've regenerated everything required.
Another possibility is that you built this new version of automake
yourself and installed it early in your path. In that case,
it is not using the same internal search path, which causes
trouble unless you've also built libtool with the same $(prefix).
In any case, I would recommend your using automake-1.11.1 or newer
for all development.
gnulib/lib/Makefile.am:21: to `configure.ac' and run
`aclocal' and
`autoconf' again.
gnulib/lib/Makefile.am:21: If `AC_PROG_LIBTOOL' is in `configure.ac',
make sure
gnulib/lib/Makefile.am:21: its definition is in aclocal's search path.
3:41 a.m.
New subject: [libvirt] patch submission: git send-email, format-patch or git diff
Cole Robinson <crobinso(a)redhat.com> wrote on 30/04/2010
15:42:05:
From: Cole Robinson <crobinso(a)redhat.com>
To: Kenneth Nagin/Haifa/IBM@IBMIL
Cc: "Daniel P. Berrange" <berrange(a)redhat.com>, list libvirt
<libvir-list(a)redhat.com>, Daniel Veillard <veillard(a)redhat.com>
Date: 30/04/2010 15:42
Subject: Re: [libvirt] (Resend) Live Migration with non-shared storage
for kvm
Finding a way to post the patch in-line will also probably get
better
attention: just pasting it into the mail client will probably mangle the
patch, I'd recommend git send-email.
- Cole
I'm new to git so I suspect that I don't understand the proper method for
patch submission. But this is the problem that I see with your suggestion.
git send-email implies usage of git-format-patch. But git-format-patch
creates a set of files one per commit.
However, don't you want to submit the diff between the changed code and
the master, i.e. git diff master > patch?
regards,
Kenneth Nagin
4:16 a.m.
New subject: [libvirt] patch submission: git send-email, format-patch or git diff
Kenneth Nagin wrote:
>Cole Robinson <crobinso(a)redhat.com> wrote on 30/04/2010
15:42:05:
> From: Cole Robinson <crobinso(a)redhat.com>
> To: Kenneth Nagin/Haifa/IBM@IBMIL
> Cc: "Daniel P. Berrange" <berrange(a)redhat.com>, list libvirt
> <libvir-list(a)redhat.com>, Daniel Veillard <veillard(a)redhat.com>
> Date: 30/04/2010 15:42
> Subject: Re: [libvirt] (Resend) Live Migration with non-shared storage
for kvm
> Finding a way to post the patch in-line will also probably get better
> attention: just pasting it into the mail client will probably mangle the
> patch, I'd recommend git send-email.
I'm new to git so I suspect that I don't understand the
proper method for
patch submission. But this is the problem that I see with your suggestion.
git send-email implies usage of git-format-patch. But git-format-patch
creates a set of files one per commit.
If (and this is a big "if") your mail client does not split lines
or mangle white space, it's ok to use e.g.,
git format-patch --stdout -1 > FILE
and then to insert FILE in the body of your message.
That will put the single, most recent change-set in FILE.
If you want to submit a series of N (say N=5) change sets,
you can s/-1/-5/:
git format-patch --stdout -5 > FILE
still assuming that the 5 most recent change sets are the ones you want.
Regarding your mail client, you should try it first
by sending yourself a patch. When you receive it, save
the patch to a file, say PATCH, removing any ">" that were prepended
to "From" lines, and then run "git checkout master; git am PATCH" and
ensure that that succeeds. If there is any problem, go back to square 1
and either fix your client or use git send-email...
Using "git send-email" is similar, but makes it easier for
the recipient/reviewer to apply your patch.
However, don't you want to submit the diff between the changed
code and
the master, i.e. git diff master > patch?
Yes. Typically I copy the 00* patch files into an empty
directory, cd into that directory and then run this:
git send-email --compose --to=libvir-list(a)redhat.com 00*
Be sure that you've set this in your ~/.gitconfig:
git config --global sendemail.chainreplyto no
8:39 a.m.
New subject: [libvirt] patch submission: git send-email, format-patch or git diff
On 05/03/2010 04:41 AM, Kenneth Nagin wrote:
> Cole Robinson <crobinso(a)redhat.com> wrote on 30/04/2010 15:42:05:
> From: Cole Robinson <crobinso(a)redhat.com>
> To: Kenneth Nagin/Haifa/IBM@IBMIL
> Cc: "Daniel P. Berrange" <berrange(a)redhat.com>, list libvirt
> <libvir-list(a)redhat.com>, Daniel Veillard <veillard(a)redhat.com>
> Date: 30/04/2010 15:42
> Subject: Re: [libvirt] (Resend) Live Migration with non-shared storage
for kvm
> Finding a way to post the patch in-line will also probably get better
> attention: just pasting it into the mail client will probably mangle the
> patch, I'd recommend git send-email.
>
>
> - Cole
>
I'm new to git so I suspect that I don't understand the proper method for
patch submission. But this is the problem that I see with your suggestion.
git send-email implies usage of git-format-patch. But git-format-patch
creates a set of files one per commit.
You can pass options to format-patch which will limit the range of
commits it will dump: git format-patch -3 will dump the latest 3 commits
for example. If your changes are spread over multiple commits and you
want to submit it all as a single change, use git rebase -i to squash
commits together.
However, don't you want to submit the diff between the changed
code and
the master, i.e. git diff master > patch?
A simplification of my workflow is:
git checkout master
git pull
git checkout -b workbranch
Hack, committing any changes along the way
git rebase -i to clean up the commits
git format-patch -#
git send-email
- Cole
9:26 a.m.
New subject: [libvirt] patch submission: git send-email, format-patch or git diff
On 05/03/2010 09:39 AM, Cole Robinson wrote:
A simplification of my workflow is:
git checkout master
git pull
git checkout -b workbranch
Hack, committing any changes along the way
git rebase -i to clean up the commits
The one thing I would add here would be:
git checkout master
git pull
git checkout workbranch
git rebase master
(fix any conflicts)
to bring your workbranch up to date with master, and make it less likely
others would encounter conflicts when they try to apply your patches to
their work tree.
git format-patch -#
git send-email
I recently learned that git send-email will send directly from the git
history, without the bother of using git format-patch to create
intermediate files. For example:
git send-email -1
will send an email containing the latest commit on the current branch.
git send-email --compose -5
will open your editor of choice to compose an email introducing the
patch series (you should put "[PATCH 0/n]" at the beginning of the
subject where n is, in this example, 5). After you've saved that mail,
it will send the introduction mail followed by a single email for each
of the last 5 commits on the current branch.
I much prefer doing it this way, because I don't end up with tons of
patchfiles lying around (which usually results in me sending the wrong
set of files).
10:13 a.m.
New subject: [libvirt] patch submission: git send-email, format-patch or git diff
On 05/03/2010 08:26 AM, Laine Stump wrote:
I recently learned that git send-email will send directly from the
git
history, without the bother of using git format-patch to create
intermediate files. For example:
git send-email -1
will send an email containing the latest commit on the current branch.
git send-email --compose -5
Another useful command line:
git send-email -5 --in-reply-to=... \
--annotate --compose --subject-prefix=PATCHv2
which lets me send a 5-patch series as a v2 reply to an earlier email,
with a cover letter, and allowing me to add comments to each individual
patch to document what changed from v1. The only thing that makes this
hard is that thunderbird 2.x used to have the mnenhy plugin to make it
very easy to get the Message-ID of any given message, but that plugin
has not been ported to thunderbird 3.0 so I have to view the source of a
message and manually find the MessageID.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:31 a.m.
New subject: [libvirt] patch submission: git send-email, format-patch or git diff
On 05/03/2010 11:13 AM, Eric Blake wrote:
On 05/03/2010 08:26 AM, Laine Stump wrote:
> I recently learned that git send-email will send directly from the git
> history, without the bother of using git format-patch to create
> intermediate files. For example:
>
> git send-email -1
>
> will send an email containing the latest commit on the current branch.
>
> git send-email --compose -5
>
Another useful command line:
git send-email -5 --in-reply-to=... \
--annotate --compose --subject-prefix=PATCHv2
which lets me send a 5-patch series as a v2 reply to an earlier email,
with a cover letter, and allowing me to add comments to each individual
patch to document what changed from v1. The only thing that makes this
hard is that thunderbird 2.x used to have the mnenhy plugin to make it
very easy to get the Message-ID of any given message, but that plugin
has not been ported to thunderbird 3.0 so I have to view the source of a
message and manually find the MessageID.
I just select View/Headers/All, then right click on the "1" next to the
"message-id" line, and select "Copy". (I wish I could select that, and
a
couple other extra headers, to be displayed when "Normal" headers are
selected. With all the time I spend using Thunderbird every day, you'd
think I'd take the time to figure out where/how to make feature requests
:-P)
5075
days inactive
5144
days old
24 comments
6 participants
participants (6)
-
Cole Robinson -
Daniel P. Berrange -
Eric Blake -
Jim Meyering -
Kenneth Nagin -
Laine Stump