I just came across this little charmer:
169 void *xenbus_dev_request_and_reply(struct xsd_sockmsg *msg)
170 {
171 void *ret;
172 struct xsd_sockmsg req_msg = *msg;
173 int err;
174
175 if (req_msg.type == XS_TRANSACTION_START)
176 down_read(&xs_state.suspend_mutex);
177
178 mutex_lock(&xs_state.request_mutex);
179
180 err = xb_write(msg, sizeof(*msg) + msg->len);
181 if (err) {
182 msg->type = XS_ERROR;
183 ret = ERR_PTR(err);
184 } else
185 ret = read_reply(&msg->type, &msg->len);
186
187 mutex_unlock(&xs_state.request_mutex);
188
189 if ((req_msg.type == XS_TRANSACTION_END) ||
190 ((req_msg.type == XS_TRANSACTION_START) &&
191 (msg->type == XS_ERROR)))
192 up_read(&xs_state.suspend_mutex);
193
194 return ret;
195 }
So if userspace gets killed during a transaction, we''ll never be able
to
suspend. And of course an invalid transaction end will panic the domain.
Unfortunately blktap is actually using transactions.
Given that transaction ID''s are local to a connection, can''t
we avoid
taking the mutex altogether? Any further requests as part of the
transaction will not be found in the list and we''ll return an error
reply as expected. Am I missing something?
regards
john
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On 2/3/07 18:11, "John Levon" <levon@movementarian.org> wrote:> So if userspace gets killed during a transaction, we''ll never be able to > suspend. And of course an invalid transaction end will panic the domain. > > Unfortunately blktap is actually using transactions.xenbus_dev_release() aborts any in-progress transactions when the device is closed (e.g., due to process crash). I don''t claim this is nice by the way. If I hasd time I would actually do transaction-id renumbering in the xenstore device driver (so application may see different number from the real physical transaction id): each application-visible identifier maps to a physical identifier or to XBT_NIL (if the transaction started before a save/restore). If it maps to XBT_NIL then reads/writes are done outside of a transaction and the transaction_end will always return ''fail''. This avoids needing to modify users of xenbus to deal with failure-return codes from reads/writes inside of transactions (which absolutely none of them do right now). Instead they''ll get back some ''reasonable'' data values and continue on to the transaction_end request, which they *can* handle failure of. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, Mar 02, 2007 at 06:33:07PM +0000, Keir Fraser wrote:> > So if userspace gets killed during a transaction, we''ll never be able to > > suspend. And of course an invalid transaction end will panic the domain. > > > > Unfortunately blktap is actually using transactions. > > xenbus_dev_release() aborts any in-progress transactions when the device is > closed (e.g., due to process crash).Ah, I see.> I don''t claim this is nice by the way. If I hasd time I would actually do > transaction-id renumbering in the xenstore device driver (so application may > see different number from the real physical transaction id): each > application-visible identifier maps to a physical identifier or to XBT_NIL > (if the transaction started before a save/restore). If it maps to XBT_NIL > then reads/writes are done outside of a transaction and the transaction_end > will always return ''fail''. This avoids needing to modify users of xenbus to > deal with failure-return codes from reads/writes inside of transactions > (which absolutely none of them do right now). Instead they''ll get back some > ''reasonable'' data values and continue on to the transaction_end request, > which they *can* handle failure of.Hmm, it would be simpler and better to just fix the users, but there''s simple way to tell that it failed due to an invalid transaction ID unfortunately, so I suppose something like this will be necessary... regards john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel