On Thu, Dec 16, 2010 at 08:27:23AM -0700, Eric Blake wrote: > On 12/16/2010 04:21 AM, Daniel P. Berrange wrote: > > +const VIR_NET_MESSAGE_HEADER_MAX = 24; > > + > > +/* Size of message payload */ > > +const VIR_NET_MESSAGE_PAYLOAD_MAX = 262120; > > Would it be better to write VIR_NET_MESSAGE_PAYLOAD_MAX = > VIR_NET_MESSAGE_MAX - VIR_NET_MESSAGE_HEADER_MAX? But that's just cosmetic.
Unfortunately you can use expressions when defining XDR constants. > > > + > > +/* Size of message length field. Not counted in VIR_NET_MESSAGE_MAX */ > > +const VIR_NET_MESSAGE_LEN_MAX = 4; > > + > > +const VIR_NET_MESSAGE_STRING_MAX = 65536; > > You lost a useful comment from remote_protocol.h: > > /* Length of long, but not unbounded, strings. > * This is an arbitrary limit designed to stop the decoder from trying > * to allocate unbounded amounts of memory when fed with a bad message. > */ Yeah, I didn't even really want these bits of the protocol, but backcompat for the error struct forced me to copy them in. > > + * and the 'status' field varies according to: > > + * > > + * - type == VIR_NET_CALL > > + * * VIR_NET_OK always > > + * > > + * - type == VIR_NET_REPLY > > + * * VIR_NET_OK if RPC finished successfully > > + * * VIR_NET_ERROR if something failed > > + * > > + * - type == VIR_NET_MESSAGE > > + * * VIR_NET_OK always > > + * > > + * - type == VIR_NET_STREAM > > + * * VIR_NET_CONTINUE if more data is following > > + * * VIR_NET_OK if stream is complete > > + * * VIR_NET_ERROR if stream had an error > > + * > > + * Payload varies according to type and status: > > + * > > + * - type == VIR_NET_CALL > > + * XXX_args for procedure > > + * > > + * - type == VIR_NET_REPLY > > + * * status == VIR_NET_OK > > + * XXX_ret for procedure > > + * * status == VIR_NET_ERROR > > + * remote_error Error information > > + * > > + * - type == VIR_NET_MESSAGE > > + * * status == VIR_NET_OK > > + * XXX_args for procedure > > + * * status == VIR_NET_ERROR > > + * remote_error Error information > > This is pure copy-and-paste from remote_protocol.x, but it doesnt' make > sense to me. Earlier, you said that VIR_NET_MESSAGE implies status is > VIR_NET_OK always, and that messages are asynchronous, rather than in > reply to a message. Either this should be: > > - type == VIR_NET_MESSAGE > XXX_msg according to proc Yeah that's the correct one. There are no errors associated with async events. > or you need to allow for VIR_NET_MESSAGE to pass status==VIR_NET_ERROR. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
