Patchwork savevm: unbreak register_savevm_live()/vmstate_register_with_alias_id()

login
register
mail settings
Submitter Isaku Yamahata
Date Jan. 26, 2011, 9:45 a.m.
Message ID <a3b18c39b5c12fc504f7f76938d9a674b35f471f.1296035051.git.yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/80497/
State New
Headers show

Comments

Isaku Yamahata - Jan. 26, 2011, 9:45 a.m.
This patch unbreaks 7685ee6abcb939104801f84b3fe9645412528088.
With the changeset, more than one instances of same device on bus
that provides get_dev_path method can't be created because it hits
the assertion.
This patch removes the assertion whose assumption isn't correct.

Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 savevm.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
Alex Williamson - Jan. 26, 2011, 2:04 p.m.
On Wed, 2011-01-26 at 18:45 +0900, Isaku Yamahata wrote:
> This patch unbreaks 7685ee6abcb939104801f84b3fe9645412528088.
> With the changeset, more than one instances of same device on bus
> that provides get_dev_path method can't be created because it hits
> the assertion.
> This patch removes the assertion whose assumption isn't correct.

Can you give an example of where this is needed?  The point of
get_dev_path is that when it's available, it's supposed to provide a
unique string per device.  Since get_dev_path is really only implemented
for PCI, does this mean you have two independent devices at the same PCI
address?  Thanks,

Alex

> Cc: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  savevm.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/savevm.c b/savevm.c
> index fcd8db4..cd29eb8 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1126,7 +1126,6 @@ int register_savevm_live(DeviceState *dev,
>      } else {
>          se->instance_id = instance_id;
>      }
> -    assert(!se->compat || se->instance_id == 0);
>      /* add at the end of list */
>      QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
>      return 0;
> @@ -1236,7 +1235,6 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
>      } else {
>          se->instance_id = instance_id;
>      }
> -    assert(!se->compat || se->instance_id == 0);
>      /* add at the end of list */
>      QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
>      return 0;
Isaku Yamahata - Jan. 27, 2011, 3:41 a.m.
I sent the patch out too early.
I found the issue is in pcibus_get_dev_path(). Sorry for noise.

On Wed, Jan 26, 2011 at 07:04:51AM -0700, Alex Williamson wrote:
> On Wed, 2011-01-26 at 18:45 +0900, Isaku Yamahata wrote:
> > This patch unbreaks 7685ee6abcb939104801f84b3fe9645412528088.
> > With the changeset, more than one instances of same device on bus
> > that provides get_dev_path method can't be created because it hits
> > the assertion.
> > This patch removes the assertion whose assumption isn't correct.
> 
> Can you give an example of where this is needed?  The point of
> get_dev_path is that when it's available, it's supposed to provide a
> unique string per device.  Since get_dev_path is really only implemented
> for PCI, does this mean you have two independent devices at the same PCI
> address?  Thanks,
> 
> Alex
> 
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > ---
> >  savevm.c |    2 --
> >  1 files changed, 0 insertions(+), 2 deletions(-)
> > 
> > diff --git a/savevm.c b/savevm.c
> > index fcd8db4..cd29eb8 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -1126,7 +1126,6 @@ int register_savevm_live(DeviceState *dev,
> >      } else {
> >          se->instance_id = instance_id;
> >      }
> > -    assert(!se->compat || se->instance_id == 0);
> >      /* add at the end of list */
> >      QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
> >      return 0;
> > @@ -1236,7 +1235,6 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
> >      } else {
> >          se->instance_id = instance_id;
> >      }
> > -    assert(!se->compat || se->instance_id == 0);
> >      /* add at the end of list */
> >      QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
> >      return 0;
> 
> 
>

Patch

diff --git a/savevm.c b/savevm.c
index fcd8db4..cd29eb8 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1126,7 +1126,6 @@  int register_savevm_live(DeviceState *dev,
     } else {
         se->instance_id = instance_id;
     }
-    assert(!se->compat || se->instance_id == 0);
     /* add at the end of list */
     QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
     return 0;
@@ -1236,7 +1235,6 @@  int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
     } else {
         se->instance_id = instance_id;
     }
-    assert(!se->compat || se->instance_id == 0);
     /* add at the end of list */
     QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
     return 0;