[ovs-dev] [PATCH 12/13] log: Use absolute name of log file.

Ben Pfaff blp at ovn.org
Mon Dec 25 02:22:35 UTC 2017


On Sat, Dec 23, 2017 at 05:52:38PM -0700, Justin Pettit wrote:
> 
> 
> > On Dec 7, 2017, at 5:12 PM, Ben Pfaff <blp at ovn.org> wrote:
> > 
> > @@ -487,6 +507,24 @@ ovsdb_log_unread(struct ovsdb_log *file)
> >     file->offset = file->prev_offset;
> > }
> > 
> > +static struct ovsdb_error *
> > +ovsdb_log_truncate(struct ovsdb_log *file)
> > +{
> > +    file->state = OVSDB_LOG_WRITE;
> > +
> > +    struct ovsdb_error *error = NULL;
> > +    if (fseeko(file->stream, file->offset, SEEK_SET)) {
> > +        error = ovsdb_io_error(errno, "%s: cannot seek to offset %lld",
> > +                               file->display_name,
> > +                               (long long int) file->offset);
> > +    } else if (ftruncate(fileno(file->stream), file->offset)) {
> > +        error = ovsdb_io_error(errno, "%s: cannot truncate to length %lld",
> > +                               file->display_name,
> > +                               (long long int) file->offset);
> > +    }
> > +    return error;
> > +}
> > ...
> > @@ -521,19 +557,13 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json)
> >     case OVSDB_LOG_READ:
> >     case OVSDB_LOG_READ_ERROR:
> >     case OVSDB_LOG_WRITE_ERROR:
> >         ovsdb_error_destroy(file->error);
> > 
> > +        file->error = ovsdb_log_truncate(file);
> > +        if (file->error) {
> > +            file->state = OVSDB_LOG_WRITE_ERROR;
> > +            return ovsdb_error_clone(file->error);
> >         }
> > +        file->state = OVSDB_LOG_WRITE;
> >         break;
> 
> Obviously not a big deal, but I think "file->state" will always be in
> OVSDB_LOG_WRITE due to the call to ovsdb_log_truncate().

Yeah, you're right.  I didn't notice that before, but I think it's
harmless or even a good thing, so I left it.

> > @@ -541,8 +571,7 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json)
> > 
> >     if (json->type != JSON_OBJECT && json->type != JSON_ARRAY) {
> > -        error = OVSDB_BUG("bad JSON type");
> > -        goto error;
> > +        return OVSDB_BUG("bad JSON type");
> 
> Does this not need to set the state to OVSDB_LOG_WRITE_ERROR any more?

This seems qualitatively different to me from other write errors--it's a
bug in the caller rather than an I/O error--so I think it's OK.  Happy
to change it later if necessary.

> Acked-by: Justin Pettit <jpettit at ovn.org>

Thanks, applied to master.


More information about the dev mailing list