On Mon, Mar 24, 2014 at 05:11:16PM +0000, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (m...@redhat.com) wrote:
> > Can be used to verify a required field exists or validate
> > state in some other way.
> >
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> > include/migration/vmstate.h | 1 +
> > vmstate.c | 10 +++++++++-
> > 2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> > index 3a1587e..eb90cef 100644
> > --- a/include/migration/vmstate.h
> > +++ b/include/migration/vmstate.h
> > @@ -101,6 +101,7 @@ enum VMStateFlags {
> > VMS_MULTIPLY = 0x200, /* multiply "size" field by field_size
> > */
> > VMS_VARRAY_UINT8 = 0x400, /* Array with size in uint8_t field*/
> > VMS_VARRAY_UINT32 = 0x800, /* Array with size in uint32_t field*/
> > + VMS_MUST_EXIST = 0x1000, /* Field must exist in input */
> > };
> >
> > typedef struct {
> > diff --git a/vmstate.c b/vmstate.c
> > index fe53735..4943b83 100644
> > --- a/vmstate.c
> > +++ b/vmstate.c
> > @@ -13,7 +13,7 @@ static int vmstate_n_elems(void *opaque, VMStateField
> > *field)
> > {
> > int n_elems = 1;
> >
> > - if (!(field->flags & ~VMS_NONE)) {
> > + if (!(field->flags & ~(VMS_NONE | VMS_MUST_EXIST))) {
> > n_elems = 0;
> > } else if (field->flags & VMS_ARRAY) {
> > n_elems = field->num;
> > @@ -107,6 +107,9 @@ int vmstate_load_state(QEMUFile *f, const
> > VMStateDescription *vmsd,
> > }
> > }
> > field++;
> > + } else if (field->flags & VMS_MUST_EXIST) {
> > + fprintf(stderr, "Input validation failed: %s/%s\n", vmsd->name,
> > field->name);
> > + return -1;
>
> I think your intent here is just to misuse the field_exist function pointer
> as a call for a different reason as a hook for a validator; is it really worth
> misusing it like that or is something more explicit worth it?
> Perhaps something passed an Error** so it could pass back what was wrong?
Well adding a required field seems valuable by itself, does it not?
And there's no way to pass in Error** since none of the callers
has Error**: all of migration still uses stderr to pass
errors.
So we could add an API but it doesn't seem too valuable.
Since all callers will use this through a wrapper like VMSTATE_TEST,
it will be easy to change our mind later.
> > }
> > ret = vmstate_subsection_load(f, vmsd, opaque);
> > if (ret != 0) {
> > @@ -148,6 +151,11 @@ void vmstate_save_state(QEMUFile *f, const
> > VMStateDescription *vmsd,
> > field->info->put(f, addr, size);
> > }
> > }
> > + } else {
> > + if (field->flags & VMS_MUST_EXIST) {
> > + fprintf(stderr, "Input validation failed: %s/%s\n",
> > vmsd->name, field->name);
> > + assert(!(field->flags & VMS_MUST_EXIST));
>
> Wrong message for the save side.
Thanks!
> Dave
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
