* [Qemu-devel] [PATCH 0/3] Protect against long IDs
@ 2017-01-09 20:13 Dr. David Alan Gilbert (git)
2017-01-09 20:13 ` [Qemu-devel] [PATCH 1/3] vmstate_register_with_alias_id: Take an Error ** Dr. David Alan Gilbert (git)
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-09 20:13 UTC (permalink / raw)
To: qemu-devel, mst, pbonzini, quintela, amit.shah
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
QEMU currently asserts if you try and create a PCI device
on the end of a very long chain, because the ID string
exceeds the maximum length, and ends up aliasing.
Fail with a clean error in this common case; there's
lots of other places that call the various registration
functions that now check for this error; I've only made
sure the common qdev path fails cleanly.
With these patches it fails with the slightly cleaner:
qemu-system-x86_64: -device x3130-upstream,id=pci.52,bus=pci.51,addr=0x0: Path too long for VMState (0000:00:0f.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0:00.0)
I don't think in real uses we'll end up with paths this long,
so I'm not intending to fix the paths to be dynamic lengths
unless we find a really good case where it happens.
This corresponds to:
https://bugzilla.redhat.com/show_bug.cgi?id=1342434
Dave
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Dr. David Alan Gilbert (3):
vmstate_register_with_alias_id: Take an Error **
migration: Check for ID length
vmstate registration: check return values
hw/core/qdev.c | 7 +++++--
hw/intc/apic_common.c | 2 +-
include/migration/vmstate.h | 7 +++++--
migration/savevm.c | 24 ++++++++++++++++++------
stubs/vmstate.c | 3 ++-
5 files changed, 31 insertions(+), 12 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 1/3] vmstate_register_with_alias_id: Take an Error **
2017-01-09 20:13 [Qemu-devel] [PATCH 0/3] Protect against long IDs Dr. David Alan Gilbert (git)
@ 2017-01-09 20:13 ` Dr. David Alan Gilbert (git)
2017-02-01 12:56 ` Juan Quintela
2017-01-09 20:13 ` [Qemu-devel] [PATCH 2/3] migration: Check for ID length Dr. David Alan Gilbert (git)
2017-01-09 20:13 ` [Qemu-devel] [PATCH 3/3] vmstate registration: check return values Dr. David Alan Gilbert (git)
2 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-09 20:13 UTC (permalink / raw)
To: qemu-devel, mst, pbonzini, quintela, amit.shah
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
I'll be adding an error to it in a subsequent patch.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/core/qdev.c | 3 ++-
hw/intc/apic_common.c | 2 +-
include/migration/vmstate.h | 5 +++--
migration/savevm.c | 3 ++-
stubs/vmstate.c | 3 ++-
5 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 5783442..ea97b15 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -935,7 +935,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
if (qdev_get_vmsd(dev)) {
vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
dev->instance_id_alias,
- dev->alias_required_for_version);
+ dev->alias_required_for_version,
+ NULL);
}
QLIST_FOREACH(bus, &dev->child_bus, sibling) {
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index d78c885..9b40af1 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -328,7 +328,7 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
instance_id = -1;
}
vmstate_register_with_alias_id(NULL, instance_id, &vmstate_apic_common,
- s, -1, 0);
+ s, -1, 0, NULL);
}
static void apic_common_unrealize(DeviceState *dev, Error **errp)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 1638ee5..73f3182 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -952,14 +952,15 @@ bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *base, int alias_id,
- int required_for_version);
+ int required_for_version,
+ Error **errp);
static inline int vmstate_register(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *opaque)
{
return vmstate_register_with_alias_id(dev, instance_id, vmsd,
- opaque, -1, 0);
+ opaque, -1, 0, NULL);
}
void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
diff --git a/migration/savevm.c b/migration/savevm.c
index 0363372..ae3ab2c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -626,7 +626,8 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *opaque, int alias_id,
- int required_for_version)
+ int required_for_version,
+ Error **errp)
{
SaveStateEntry *se;
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index 6590627..bbe158f 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -8,7 +8,8 @@ int vmstate_register_with_alias_id(DeviceState *dev,
int instance_id,
const VMStateDescription *vmsd,
void *base, int alias_id,
- int required_for_version)
+ int required_for_version,
+ Error **errp)
{
return 0;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 2/3] migration: Check for ID length
2017-01-09 20:13 [Qemu-devel] [PATCH 0/3] Protect against long IDs Dr. David Alan Gilbert (git)
2017-01-09 20:13 ` [Qemu-devel] [PATCH 1/3] vmstate_register_with_alias_id: Take an Error ** Dr. David Alan Gilbert (git)
@ 2017-01-09 20:13 ` Dr. David Alan Gilbert (git)
2017-02-01 12:57 ` Juan Quintela
2017-01-09 20:13 ` [Qemu-devel] [PATCH 3/3] vmstate registration: check return values Dr. David Alan Gilbert (git)
2 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-09 20:13 UTC (permalink / raw)
To: qemu-devel, mst, pbonzini, quintela, amit.shah
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
The qdev id of a device can be huge if it's on the end of a chain
of bridges; in reality such chains shouldn't occur but they can
be made to by chaining PCIe bridges together.
The migration format has a number of 256 character long format
limits; check we don't hit them (we already use pstrcat/cpy but
that just protects us from buffer overruns, we fairly quickly
hit an assert).
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
include/migration/vmstate.h | 2 ++
migration/savevm.c | 21 ++++++++++++++++-----
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 73f3182..93b4722 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -949,12 +949,14 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
+/* Returns: 0 on success, -1 on failure */
int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *base, int alias_id,
int required_for_version,
Error **errp);
+/* Returns: 0 on success, -1 on failure */
static inline int vmstate_register(DeviceState *dev, int instance_id,
const VMStateDescription *vmsd,
void *opaque)
diff --git a/migration/savevm.c b/migration/savevm.c
index ae3ab2c..84324b2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -559,8 +559,14 @@ int register_savevm_live(DeviceState *dev,
if (dev) {
char *id = qdev_get_dev_path(dev);
if (id) {
- pstrcpy(se->idstr, sizeof(se->idstr), id);
- pstrcat(se->idstr, sizeof(se->idstr), "/");
+ if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >=
+ sizeof(se->idstr)) {
+ error_report("Path too long for VMState (%s)", id);
+ g_free(id);
+ g_free(se);
+
+ return -1;
+ }
g_free(id);
se->compat = g_new0(CompatEntry, 1);
@@ -644,9 +650,14 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
if (dev) {
char *id = qdev_get_dev_path(dev);
if (id) {
- pstrcpy(se->idstr, sizeof(se->idstr), id);
- pstrcat(se->idstr, sizeof(se->idstr), "/");
- g_free(id);
+ if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >=
+ sizeof(se->idstr)) {
+ error_setg(errp, "Path too long for VMState (%s)", id);
+ g_free(id);
+ g_free(se);
+
+ return -1;
+ }
se->compat = g_new0(CompatEntry, 1);
pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), vmsd->name);
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 3/3] vmstate registration: check return values
2017-01-09 20:13 [Qemu-devel] [PATCH 0/3] Protect against long IDs Dr. David Alan Gilbert (git)
2017-01-09 20:13 ` [Qemu-devel] [PATCH 1/3] vmstate_register_with_alias_id: Take an Error ** Dr. David Alan Gilbert (git)
2017-01-09 20:13 ` [Qemu-devel] [PATCH 2/3] migration: Check for ID length Dr. David Alan Gilbert (git)
@ 2017-01-09 20:13 ` Dr. David Alan Gilbert (git)
2017-01-09 21:39 ` Peter Maydell
2017-02-01 12:58 ` Juan Quintela
2 siblings, 2 replies; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-09 20:13 UTC (permalink / raw)
To: qemu-devel, mst, pbonzini, quintela, amit.shah
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Check qdev's call to vmstate_register_with_alias_id; that gets
most of the common uses; there's hundreds of calls via vmstate_register
which could get fixed over time.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/core/qdev.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index ea97b15..df633d0 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -933,10 +933,12 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
}
if (qdev_get_vmsd(dev)) {
- vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
+ if (vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
dev->instance_id_alias,
dev->alias_required_for_version,
- NULL);
+ &local_err) < 0) {
+ goto post_realize_fail;
+ }
}
QLIST_FOREACH(bus, &dev->child_bus, sibling) {
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vmstate registration: check return values
2017-01-09 20:13 ` [Qemu-devel] [PATCH 3/3] vmstate registration: check return values Dr. David Alan Gilbert (git)
@ 2017-01-09 21:39 ` Peter Maydell
2017-01-10 9:26 ` Dr. David Alan Gilbert
2017-02-01 12:58 ` Juan Quintela
1 sibling, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2017-01-09 21:39 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: QEMU Developers, Michael S. Tsirkin, Paolo Bonzini,
Juan Quintela, Amit Shah
On 9 January 2017 at 20:13, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Check qdev's call to vmstate_register_with_alias_id; that gets
> most of the common uses; there's hundreds of calls via vmstate_register
> which could get fixed over time.
Not quite that bad, I think -- I make it just over 50 calls.
thanks
-- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vmstate registration: check return values
2017-01-09 21:39 ` Peter Maydell
@ 2017-01-10 9:26 ` Dr. David Alan Gilbert
2017-01-10 10:10 ` Peter Maydell
0 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2017-01-10 9:26 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, Michael S. Tsirkin, Paolo Bonzini,
Juan Quintela, Amit Shah
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 9 January 2017 at 20:13, Dr. David Alan Gilbert (git)
> <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Check qdev's call to vmstate_register_with_alias_id; that gets
> > most of the common uses; there's hundreds of calls via vmstate_register
> > which could get fixed over time.
>
> Not quite that bad, I think -- I make it just over 50 calls.
Well kind of; it seems to be a bit more complicated than that.
I'd grep'd for vmstate_register and that gives me ~180 (including
stuff in headers).
Only 56 of those are vmstate_register() calls though, 117 are
vmstate_register_ram calls which I'd not previously looked at,
those call qemu_ram_set_idstr which looks like it suffers from
the same problem though.
Dave
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vmstate registration: check return values
2017-01-10 9:26 ` Dr. David Alan Gilbert
@ 2017-01-10 10:10 ` Peter Maydell
2017-01-10 10:34 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2017-01-10 10:10 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: QEMU Developers, Michael S. Tsirkin, Paolo Bonzini,
Juan Quintela, Amit Shah
On 10 January 2017 at 09:26, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Peter Maydell (peter.maydell@linaro.org) wrote:
>> On 9 January 2017 at 20:13, Dr. David Alan Gilbert (git)
>> <dgilbert@redhat.com> wrote:
>> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> >
>> > Check qdev's call to vmstate_register_with_alias_id; that gets
>> > most of the common uses; there's hundreds of calls via vmstate_register
>> > which could get fixed over time.
>>
>> Not quite that bad, I think -- I make it just over 50 calls.
>
> Well kind of; it seems to be a bit more complicated than that.
> I'd grep'd for vmstate_register and that gives me ~180 (including
> stuff in headers).
Yes, I was specifically looking at the vmstate_register and
vmstate_register_with_alias_id ones.
> Only 56 of those are vmstate_register() calls though, 117 are
> vmstate_register_ram calls which I'd not previously looked at,
> those call qemu_ram_set_idstr which looks like it suffers from
> the same problem though.
They call qemu_ram_set_idstr with the memory region name string,
though, which is "used for debugging; not visible to the user
or ABI", so we can just say it's a bug to use a silly name
and assert if it's too big, right?
thanks
-- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vmstate registration: check return values
2017-01-10 10:10 ` Peter Maydell
@ 2017-01-10 10:34 ` Dr. David Alan Gilbert
2017-01-10 10:44 ` Peter Maydell
0 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2017-01-10 10:34 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, Michael S. Tsirkin, Paolo Bonzini,
Juan Quintela, Amit Shah
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 10 January 2017 at 09:26, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> >> On 9 January 2017 at 20:13, Dr. David Alan Gilbert (git)
> >> <dgilbert@redhat.com> wrote:
> >> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >> >
> >> > Check qdev's call to vmstate_register_with_alias_id; that gets
> >> > most of the common uses; there's hundreds of calls via vmstate_register
> >> > which could get fixed over time.
> >>
> >> Not quite that bad, I think -- I make it just over 50 calls.
> >
> > Well kind of; it seems to be a bit more complicated than that.
> > I'd grep'd for vmstate_register and that gives me ~180 (including
> > stuff in headers).
>
> Yes, I was specifically looking at the vmstate_register and
> vmstate_register_with_alias_id ones.
>
> > Only 56 of those are vmstate_register() calls though, 117 are
> > vmstate_register_ram calls which I'd not previously looked at,
> > those call qemu_ram_set_idstr which looks like it suffers from
> > the same problem though.
>
> They call qemu_ram_set_idstr with the memory region name string,
> though, which is "used for debugging; not visible to the user
> or ABI", so we can just say it's a bug to use a silly name
> and assert if it's too big, right?
qemu_ram_set_idstr already abort's if it hits a dupe (which after
making sure it doesn't overflow the buffer is what we end up with
if we have long names); so yes we already abort in that case.
However, it's a bit optimistic of the memory region to claim the name
is just for debug; Migration/ram.c transmits the RAMBlock's idstr on
the wire (as does postcopy) - so I think the memory.h comment
is wrong.
I don't think it's a big problem since you're unlikely to hit these
big names in practice; but it would be better to return an error
rather than assert/abort since then you wouldn't abort as part
of a hot-add.
So it's worth taking the common cases as this patch does; I don't
think it's worth the hastle of changing 100+ calls though.
Dave
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vmstate registration: check return values
2017-01-10 10:34 ` Dr. David Alan Gilbert
@ 2017-01-10 10:44 ` Peter Maydell
0 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2017-01-10 10:44 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: QEMU Developers, Michael S. Tsirkin, Paolo Bonzini,
Juan Quintela, Amit Shah
On 10 January 2017 at 10:34, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> However, it's a bit optimistic of the memory region to claim the name
> is just for debug; Migration/ram.c transmits the RAMBlock's idstr on
> the wire (as does postcopy) - so I think the memory.h comment
> is wrong.
We'd better fix that, then, or we'll find ourselves breaking
migration compat by accident...
> I don't think it's a big problem since you're unlikely to hit these
> big names in practice; but it would be better to return an error
> rather than assert/abort since then you wouldn't abort as part
> of a hot-add.
Almost all of the calls aren't going to be hot-add, though.
> So it's worth taking the common cases as this patch does; I don't
> think it's worth the hastle of changing 100+ calls though.
You also have the code paths via memory_region_allocate_system_memory
which end up calling vmstate_register_ram_global which then calls
qemu_ram_set_idstr -- none of that has support for returning an
error.
(Aside: at some point I want to introduce a
memory_region_allocate_aux_memory() which wraps the common
pattern
memory_region_init_ram(mr, NULL, name, size, &error_fatal);
vmstate_register_ram_global(mr);
so we have a simple way to create RAM blocks which aren't
the main system ram, by analogy with memory_region_allocate_system_memory().)
thanks
-- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] vmstate_register_with_alias_id: Take an Error **
2017-01-09 20:13 ` [Qemu-devel] [PATCH 1/3] vmstate_register_with_alias_id: Take an Error ** Dr. David Alan Gilbert (git)
@ 2017-02-01 12:56 ` Juan Quintela
0 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2017-02-01 12:56 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, mst, pbonzini, amit.shah
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> I'll be adding an error to it in a subsequent patch.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] migration: Check for ID length
2017-01-09 20:13 ` [Qemu-devel] [PATCH 2/3] migration: Check for ID length Dr. David Alan Gilbert (git)
@ 2017-02-01 12:57 ` Juan Quintela
0 siblings, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2017-02-01 12:57 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, mst, pbonzini, amit.shah
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The qdev id of a device can be huge if it's on the end of a chain
> of bridges; in reality such chains shouldn't occur but they can
> be made to by chaining PCIe bridges together.
>
> The migration format has a number of 256 character long format
> limits; check we don't hit them (we already use pstrcat/cpy but
> that just protects us from buffer overruns, we fairly quickly
> hit an assert).
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vmstate registration: check return values
2017-01-09 20:13 ` [Qemu-devel] [PATCH 3/3] vmstate registration: check return values Dr. David Alan Gilbert (git)
2017-01-09 21:39 ` Peter Maydell
@ 2017-02-01 12:58 ` Juan Quintela
1 sibling, 0 replies; 12+ messages in thread
From: Juan Quintela @ 2017-02-01 12:58 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, mst, pbonzini, amit.shah
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Check qdev's call to vmstate_register_with_alias_id; that gets
> most of the common uses; there's hundreds of calls via vmstate_register
> which could get fixed over time.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/core/qdev.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index ea97b15..df633d0 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -933,10 +933,12 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
> }
>
> if (qdev_get_vmsd(dev)) {
> - vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
> + if (vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
> dev->instance_id_alias,
Indent this correctly, please.
> dev->alias_required_for_version,
> - NULL);
> + &local_err) < 0) {
> + goto post_realize_fail;
> + }
> }
>
> QLIST_FOREACH(bus, &dev->child_bus, sibling) {
Once that is fixed, I am ok with it.
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-02-01 12:59 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-09 20:13 [Qemu-devel] [PATCH 0/3] Protect against long IDs Dr. David Alan Gilbert (git)
2017-01-09 20:13 ` [Qemu-devel] [PATCH 1/3] vmstate_register_with_alias_id: Take an Error ** Dr. David Alan Gilbert (git)
2017-02-01 12:56 ` Juan Quintela
2017-01-09 20:13 ` [Qemu-devel] [PATCH 2/3] migration: Check for ID length Dr. David Alan Gilbert (git)
2017-02-01 12:57 ` Juan Quintela
2017-01-09 20:13 ` [Qemu-devel] [PATCH 3/3] vmstate registration: check return values Dr. David Alan Gilbert (git)
2017-01-09 21:39 ` Peter Maydell
2017-01-10 9:26 ` Dr. David Alan Gilbert
2017-01-10 10:10 ` Peter Maydell
2017-01-10 10:34 ` Dr. David Alan Gilbert
2017-01-10 10:44 ` Peter Maydell
2017-02-01 12:58 ` Juan Quintela
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.