| Message ID | 1309448777-1447-2-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 30, 2011 at 05:46:14PM +0200, Paolo Bonzini wrote: > We need to provide a new migration format, and not break migration > in old machine models. So add a migration_format field to QEMUMachine. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Should not machine describe guest behaviour? It seems that a flag to the migrate/save command to control format would be better - this way the same machine can migrate to different formats. > --- > cpu-common.h | 3 --- > hw/boards.h | 1 + > qemu-common.h | 3 +++ > savevm.c | 7 +++++-- > 4 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/cpu-common.h b/cpu-common.h > index b027e43..8c06dbb 100644 > --- a/cpu-common.h > +++ b/cpu-common.h > @@ -26,9 +26,6 @@ enum device_endian { > DEVICE_LITTLE_ENDIAN, > }; > > -/* address in the RAM (different from a physical address) */ > -typedef unsigned long ram_addr_t; > - > /* memory API */ > > typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value); > diff --git a/hw/boards.h b/hw/boards.h > index 716fd7b..560dbaf 100644 > --- a/hw/boards.h > +++ b/hw/boards.h > @@ -19,6 +19,7 @@ typedef struct QEMUMachine { > QEMUMachineInitFunc *init; > int use_scsi; > int max_cpus; > + unsigned migration_format; > unsigned int no_serial:1, > no_parallel:1, > use_virtcon:1, > diff --git a/qemu-common.h b/qemu-common.h > index 109498d..550fe2c 100644 > --- a/qemu-common.h > +++ b/qemu-common.h > @@ -119,6 +119,9 @@ static inline char *realpath(const char *path, char *resolved_path) > #define PRIo64 "I64o" > #endif > > +/* address in the RAM (different from a physical address) */ > +typedef unsigned long ram_addr_t; > + > /* FIXME: Remove NEED_CPU_H. */ > #ifndef NEED_CPU_H > > diff --git a/savevm.c b/savevm.c > index 8139bc7..74e6e99 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -72,6 +72,7 @@ > #include "qemu-common.h" > #include "hw/hw.h" > #include "hw/qdev.h" > +#include "hw/boards.h" > #include "net.h" > #include "monitor.h" > #include "sysemu.h" > @@ -1474,7 +1475,7 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable, > } > > qemu_put_be32(f, QEMU_VM_FILE_MAGIC); > - qemu_put_be32(f, QEMU_VM_FILE_VERSION); > + qemu_put_be32(f, current_machine->migration_format ?: QEMU_VM_FILE_VERSION); > > QTAILQ_FOREACH(se, &savevm_handlers, entry) { > int len; > @@ -1747,8 +1748,10 @@ int qemu_loadvm_state(QEMUFile *f) > fprintf(stderr, "SaveVM v2 format is obsolete and don't work anymore\n"); > return -ENOTSUP; > } > - if (v != QEMU_VM_FILE_VERSION) > + if (v != (current_machine->migration_format ?: QEMU_VM_FILE_VERSION)) { > + fprintf(stderr, "Mismatching SaveVM format v%d\n", v); > return -ENOTSUP; > + } > > while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) { > uint32_t instance_id, version_id, section_id; > -- > 1.7.5.2 > >
On 06/30/2011 08:11 PM, Michael S. Tsirkin wrote: > > Signed-off-by: Paolo Bonzini<pbonzini@redhat.com> > > Should not machine describe guest behaviour? > It seems that a flag to the migrate/save command to control format > would be better - this way the same machine can > migrate to different formats. I thought about it, but there is no savevm state variable (only migration state, but it's unused for snapshotting). So it seemed too much work for an RFC series I wanted to push out fast. Besides, you need to store a default somewhere, and the default must be the old format for versioned machine models; perhaps not from QEMU where new->old migration is a wild bet anyway, but for downstream it must. Then I think that you are suggesting a superset of this patch. I'm not even sure it is necessary to have it though: it is a correctness fix, it should be on for everyone. Paolo
On 06/30/2011 10:46 AM, Paolo Bonzini wrote: > We need to provide a new migration format, and not break migration > in old machine models. So add a migration_format field to QEMUMachine. > > Signed-off-by: Paolo Bonzini<pbonzini@redhat.com> > --- > cpu-common.h | 3 --- > hw/boards.h | 1 + > qemu-common.h | 3 +++ > savevm.c | 7 +++++-- > 4 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/cpu-common.h b/cpu-common.h > index b027e43..8c06dbb 100644 > --- a/cpu-common.h > +++ b/cpu-common.h > @@ -26,9 +26,6 @@ enum device_endian { > DEVICE_LITTLE_ENDIAN, > }; > > -/* address in the RAM (different from a physical address) */ > -typedef unsigned long ram_addr_t; > - > /* memory API */ > > typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value); > diff --git a/hw/boards.h b/hw/boards.h > index 716fd7b..560dbaf 100644 > --- a/hw/boards.h > +++ b/hw/boards.h > @@ -19,6 +19,7 @@ typedef struct QEMUMachine { > QEMUMachineInitFunc *init; > int use_scsi; > int max_cpus; > + unsigned migration_format; > unsigned int no_serial:1, > no_parallel:1, > use_virtcon:1, > diff --git a/qemu-common.h b/qemu-common.h > index 109498d..550fe2c 100644 > --- a/qemu-common.h > +++ b/qemu-common.h > @@ -119,6 +119,9 @@ static inline char *realpath(const char *path, char *resolved_path) > #define PRIo64 "I64o" > #endif > > +/* address in the RAM (different from a physical address) */ > +typedef unsigned long ram_addr_t; > + > /* FIXME: Remove NEED_CPU_H. */ > #ifndef NEED_CPU_H > > diff --git a/savevm.c b/savevm.c > index 8139bc7..74e6e99 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -72,6 +72,7 @@ > #include "qemu-common.h" > #include "hw/hw.h" > #include "hw/qdev.h" > +#include "hw/boards.h" > #include "net.h" > #include "monitor.h" > #include "sysemu.h" > @@ -1474,7 +1475,7 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable, > } > > qemu_put_be32(f, QEMU_VM_FILE_MAGIC); > - qemu_put_be32(f, QEMU_VM_FILE_VERSION); > + qemu_put_be32(f, current_machine->migration_format ?: QEMU_VM_FILE_VERSION); Please avoid this gcc extension as it's relatively obscure. But in addition, why would use you 0 as the new format instead of QEMU_VM_FILE_VERSION + 1? Regards, Anthony Liguori > QTAILQ_FOREACH(se,&savevm_handlers, entry) { > int len; > @@ -1747,8 +1748,10 @@ int qemu_loadvm_state(QEMUFile *f) > fprintf(stderr, "SaveVM v2 format is obsolete and don't work anymore\n"); > return -ENOTSUP; > } > - if (v != QEMU_VM_FILE_VERSION) > + if (v != (current_machine->migration_format ?: QEMU_VM_FILE_VERSION)) { > + fprintf(stderr, "Mismatching SaveVM format v%d\n", v); > return -ENOTSUP; > + } > > while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) { > uint32_t instance_id, version_id, section_id;
On 07/29/2011 03:08 PM, Anthony Liguori wrote: > > Please avoid this gcc extension as it's relatively obscure. But in > addition, why would use you 0 as the new format instead of > QEMU_VM_FILE_VERSION + 1? 0 is the default. If a machine doesn't specify a format, it gets the newest one automatically. Paolo
diff --git a/cpu-common.h b/cpu-common.h index b027e43..8c06dbb 100644 --- a/cpu-common.h +++ b/cpu-common.h @@ -26,9 +26,6 @@ enum device_endian { DEVICE_LITTLE_ENDIAN, }; -/* address in the RAM (different from a physical address) */ -typedef unsigned long ram_addr_t; - /* memory API */ typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value); diff --git a/hw/boards.h b/hw/boards.h index 716fd7b..560dbaf 100644 --- a/hw/boards.h +++ b/hw/boards.h @@ -19,6 +19,7 @@ typedef struct QEMUMachine { QEMUMachineInitFunc *init; int use_scsi; int max_cpus; + unsigned migration_format; unsigned int no_serial:1, no_parallel:1, use_virtcon:1, diff --git a/qemu-common.h b/qemu-common.h index 109498d..550fe2c 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -119,6 +119,9 @@ static inline char *realpath(const char *path, char *resolved_path) #define PRIo64 "I64o" #endif +/* address in the RAM (different from a physical address) */ +typedef unsigned long ram_addr_t; + /* FIXME: Remove NEED_CPU_H. */ #ifndef NEED_CPU_H diff --git a/savevm.c b/savevm.c index 8139bc7..74e6e99 100644 --- a/savevm.c +++ b/savevm.c @@ -72,6 +72,7 @@ #include "qemu-common.h" #include "hw/hw.h" #include "hw/qdev.h" +#include "hw/boards.h" #include "net.h" #include "monitor.h" #include "sysemu.h" @@ -1474,7 +1475,7 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable, } qemu_put_be32(f, QEMU_VM_FILE_MAGIC); - qemu_put_be32(f, QEMU_VM_FILE_VERSION); + qemu_put_be32(f, current_machine->migration_format ?: QEMU_VM_FILE_VERSION); QTAILQ_FOREACH(se, &savevm_handlers, entry) { int len; @@ -1747,8 +1748,10 @@ int qemu_loadvm_state(QEMUFile *f) fprintf(stderr, "SaveVM v2 format is obsolete and don't work anymore\n"); return -ENOTSUP; } - if (v != QEMU_VM_FILE_VERSION) + if (v != (current_machine->migration_format ?: QEMU_VM_FILE_VERSION)) { + fprintf(stderr, "Mismatching SaveVM format v%d\n", v); return -ENOTSUP; + } while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) { uint32_t instance_id, version_id, section_id;
We need to provide a new migration format, and not break migration in old machine models. So add a migration_format field to QEMUMachine. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- cpu-common.h | 3 --- hw/boards.h | 1 + qemu-common.h | 3 +++ savevm.c | 7 +++++-- 4 files changed, 9 insertions(+), 5 deletions(-)