Re: [Qemu-devel] [PATCH 2/3] tests: add nbd-fault-injector.py utility

[Stefan Hajnoczi]

On Tue, Feb 25, 2014 at 11:45:11AM +0000, Nick Thomas wrote:
> On 25/02/14 10:09, Stefan Hajnoczi wrote:
> > +    def send(self, buf, event):
> > +        self.check(event, 'write', 'before')
> > +        self.sock.sendall(buf)
> > +        self.check(event, 'write', 'after')
> > +
> > +    def recv(self, bufsize, event):
> > +        self.check(event, 'read', 'before')
> > +        data = recvall(self.sock, bufsize)
> > +        self.check(event, 'read', 'after')
> > +        return data
> 
> There's a class of error I recently encountered in our out-of-tree proxy
> component that only shows up if a read or write is interrupted partway
> through. Perhaps you could have a "during" event here that happens after
> bufsize/2 bytes is written?
> 
> I've not looked at qemu's block/nbd code recently, so I don't know if
> that exercises a particular failure path.

Yes, it can involve different code paths in the client.  For example,
the client may receive fields in separate recv(2) syscalls so there may
be a unique path for each field.

I think the easiest approach is to turn the 'when' option into a byte
count.  The connection will be terminated after the given number of
bytes.

Then 'before' becomes an alias for 0.  'after' becomes an alias for -1,
the magic value for the entire data length.

> > +    def close(self):
> > +        self.sock.close()
> > +
> > +def negotiate(conn):
> > +    '''Negotiate export with client'''
> > +    # Send negotiation part 1
> > +    buf = neg1_struct.pack(NBD_PASSWD, NBD_OPTS_MAGIC, 0)
> > +    conn.send(buf, event='neg1')
> > +
> > +    # Receive export option
> > +    buf = conn.recv(export_struct.size, event='export')
> > +    export = export_tuple._make(export_struct.unpack(buf))
> > +    assert export.magic == NBD_OPTS_MAGIC
> > +    assert export.opt == NBD_OPT_EXPORT_NAME
> > +    name = conn.recv(export.len, event='export-name')
> > +
> > +    # Send negotiation part 2
> > +    buf = neg2_struct.pack(8 * 1024 * 1024 * 1024, 0) # 8 GB capacity
> > +    conn.send(buf, event='neg2')
> 
> Is it worth exercising the old-style negotiation too?

Yes, probably a good idea.