| Message ID | 1412358473-31398-23-git-send-email-dgilbert@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 03, 2014 at 06:47:28PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > QEMU_VM_CMD_PACKAGED is a migration command that allows a chunk > of migration stream to be sent in one go, and be received by > a separate instance of the loadvm loop while not interacting > with the migration stream. > > This is used by postcopy to load device state (from the package) > while loading memory pages from the main stream. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Though one minor comment: [snip] > +/* We have a buffer of data to send; we don't want that all to be loaded > + * by the command itself, so the command contains just the length of the > + * extra buffer that we then send straight after it. > + * TODO: Must be a better way to organise that I'm not quite understanding what that comment's getting at.
* David Gibson (david@gibson.dropbear.id.au) wrote: > On Fri, Oct 03, 2014 at 06:47:28PM +0100, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > QEMU_VM_CMD_PACKAGED is a migration command that allows a chunk > > of migration stream to be sent in one go, and be received by > > a separate instance of the loadvm loop while not interacting > > with the migration stream. > > > > This is used by postcopy to load device state (from the package) > > while loading memory pages from the main stream. > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > Though one minor comment: > > [snip] > > +/* We have a buffer of data to send; we don't want that all to be loaded > > + * by the command itself, so the command contains just the length of the > > + * extra buffer that we then send straight after it. > > + * TODO: Must be a better way to organise that > > I'm not quite understanding what that comment's getting at. We have these VM Commands; and they are a command type, and a length: CMD_whatever length: whatever data for whatever This comment is describing that, to make things easier for this code it's ended up as: CMD_PACKAGED CMD length: 4 <--- i.e. just enough to hold the next 'length' field package length --------------- The package Which is a little different, hence i thought it needed the comment. Dave > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Tue, Nov 04, 2014 at 10:19:15AM +0000, Dr. David Alan Gilbert wrote: > * David Gibson (david@gibson.dropbear.id.au) wrote: > > On Fri, Oct 03, 2014 at 06:47:28PM +0100, Dr. David Alan Gilbert (git) wrote: > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > > > QEMU_VM_CMD_PACKAGED is a migration command that allows a chunk > > > of migration stream to be sent in one go, and be received by > > > a separate instance of the loadvm loop while not interacting > > > with the migration stream. > > > > > > This is used by postcopy to load device state (from the package) > > > while loading memory pages from the main stream. > > > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > > > Though one minor comment: > > > > [snip] > > > +/* We have a buffer of data to send; we don't want that all to be loaded > > > + * by the command itself, so the command contains just the length of the > > > + * extra buffer that we then send straight after it. > > > + * TODO: Must be a better way to organise that > > > > I'm not quite understanding what that comment's getting at. > > We have these VM Commands; and they are a command type, and a length: > CMD_whatever > length: whatever > data for whatever > > This comment is describing that, to make things easier for this code it's > ended up as: > > CMD_PACKAGED > CMD length: 4 <--- i.e. just enough to hold the next 'length' field > package length > --------------- > The package > > Which is a little different, hence i thought it needed the comment. Ah.. right. That seems.. gratuitously easy to get wrong further down the track. Why not just use the cmd length as the package length.
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 102dd93..ef98fa9 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -87,6 +87,7 @@ enum qemu_vm_cmd { QEMU_VM_CMD_INVALID = 0, /* Must be 0 */ QEMU_VM_CMD_OPENRP, /* Tell the dest to open the Return path */ QEMU_VM_CMD_REQACK, /* Request an ACK on the RP */ + QEMU_VM_CMD_PACKAGED, /* Send a wrapped stream within this stream */ QEMU_VM_CMD_POSTCOPY_RAM_ADVISE = 20, /* Prior to any page transfers, just warn we might want to do PC */ @@ -101,6 +102,8 @@ enum qemu_vm_cmd { QEMU_VM_CMD_AFTERLASTVALID }; +#define MAX_VM_CMD_PACKAGED_SIZE (1ul << 24) + bool qemu_savevm_state_blocked(Error **errp); void qemu_savevm_state_begin(QEMUFile *f, const MigrationParams *params); @@ -112,6 +115,7 @@ void qemu_savevm_command_send(QEMUFile *f, enum qemu_vm_cmd command, uint16_t len, uint8_t *data); void qemu_savevm_send_reqack(QEMUFile *f, uint32_t value); void qemu_savevm_send_openrp(QEMUFile *f); +void qemu_savevm_send_packaged(QEMUFile *f, const QEMUSizedBuffer *qsb); void qemu_savevm_send_postcopy_ram_advise(QEMUFile *f); void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name, uint16_t len, uint8_t offset, diff --git a/savevm.c b/savevm.c index b942e8c..bffe890 100644 --- a/savevm.c +++ b/savevm.c @@ -626,6 +626,38 @@ void qemu_savevm_send_openrp(QEMUFile *f) qemu_savevm_command_send(f, QEMU_VM_CMD_OPENRP, 0, NULL); } +/* We have a buffer of data to send; we don't want that all to be loaded + * by the command itself, so the command contains just the length of the + * extra buffer that we then send straight after it. + * TODO: Must be a better way to organise that + */ +void qemu_savevm_send_packaged(QEMUFile *f, const QEMUSizedBuffer *qsb) +{ + size_t cur_iov; + size_t len = qsb_get_length(qsb); + uint32_t tmp; + + tmp = cpu_to_be32(len); + + DPRINTF("send_packaged"); + qemu_savevm_command_send(f, QEMU_VM_CMD_PACKAGED, 4, (uint8_t *)&tmp); + + /* all the data follows (concatinating the iov's) */ + for (cur_iov = 0; cur_iov < qsb->n_iov; cur_iov++) { + /* The iov entries are partially filled */ + size_t towrite = (qsb->iov[cur_iov].iov_len > len) ? + len : + qsb->iov[cur_iov].iov_len; + len -= towrite; + + if (!towrite) { + break; + } + + qemu_put_buffer(f, qsb->iov[cur_iov].iov_base, towrite); + } +} + /* Send prior to any RAM transfer */ void qemu_savevm_send_postcopy_ram_advise(QEMUFile *f) { @@ -1249,6 +1281,48 @@ static int loadvm_process_command_simple_lencheck(const char *name, return 0; } +/* Immediately following this command is a blob of data containing an embedded + * chunk of migration stream; read it and load it. + */ +static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, + uint32_t length, + LoadStateEntry_Head *loadvm_handlers) +{ + int ret; + uint8_t *buffer; + QEMUSizedBuffer *qsb; + + DPRINTF("loadvm_handle_cmd_packaged: length=%u", length); + + if (length > MAX_VM_CMD_PACKAGED_SIZE) { + error_report("Unreasonably large packaged state: %u", length); + return -1; + } + buffer = g_malloc0(length); + ret = qemu_get_buffer(mis->file, buffer, (int)length); + if (ret != length) { + g_free(buffer); + error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%d\n", + ret, length); + return (ret < 0) ? ret : -EAGAIN; + } + DPRINTF("%s: Received %d package, going to load", __func__, ret); + + /* Setup a dummy QEMUFile that actually reads from the buffer */ + qsb = qsb_create(buffer, length); + g_free(buffer); /* Because qsb_create copies */ + if (!qsb) { + error_report("Unable to create qsb"); + } + QEMUFile *packf = qemu_bufopen("r", qsb); + + ret = qemu_loadvm_state_main(packf, loadvm_handlers); + DPRINTF("%s: qemu_loadvm_state_main returned %d", __func__, ret); + qemu_fclose(packf); /* also frees the qsb */ + + return ret; +} + /* * Process an incoming 'QEMU_VM_COMMAND' * negative return on error (will issue error message) @@ -1299,6 +1373,14 @@ static int loadvm_process_command(QEMUFile *f, migrate_send_rp_ack(mis, tmp32); break; + case QEMU_VM_CMD_PACKAGED: + if (loadvm_process_command_simple_lencheck("CMD_POSTCOPY_RAM_PACKAGED", + len, 4)) { + return -1; + } + tmp32 = qemu_get_be32(f); + return loadvm_handle_cmd_packaged(mis, tmp32, loadvm_handlers); + case QEMU_VM_CMD_POSTCOPY_RAM_ADVISE: if (loadvm_process_command_simple_lencheck("CMD_POSTCOPY_RAM_ADVISE", len, 16)) {