summary |
shortlog |
log |
commit | commitdiff |
tree
raw |
patch |
inline | side by side (from parent 1:
33b6381)
In the case where both ends cancel at the same time (eg. both ends
realize there are errors before or during the transfer), previously we
skipped sending back an error from the daemon, on the spurious basis
that the library would not need it (the library is cancelling because
of its own error).
However this is wrong: we should always send back an error message
from the daemon in order to preserve synchronization of the protocol.
A simple test case is:
$ guestfish -N fs -m /dev/sda1 upload nosuchfile /
libguestfs: error: open: nosuchfile: No such file or directory
libguestfs: error: unexpected procedure number (66/282)
(Notice two things: there are errors at both ends, and the
loss of synchronization).
After applying this commit, the loss of synchronization does not occur
and we just see the library error:
$ guestfish -N fs -m /dev/sda1 upload nosuchfile /
libguestfs: error: open: nosuchfile: No such file or directory
The choice of displaying the library or the daemon error is fairly
arbitrary in this case -- it would be valid to display either or even
to combine them into one error. Displaying the library error only
makes the code considerably simpler.
This commit also (re-)enables a test for this case.
return -1;
}
if (r == -2) { /* cancellation from library */
return -1;
}
if (r == -2) { /* cancellation from library */
+ /* This error is ignored by the library since it initiated the
+ * cancel. Nevertheless we must send an error reply here.
+ */
+ reply_with_error ("file upload cancelled");
- /* Do NOT send any error. */
return -1;
}
if (r == -2) { /* cancellation from library */
return -1;
}
if (r == -2) { /* cancellation from library */
+ /* This error is ignored by the library since it initiated the
+ * cancel. Nevertheless we must send an error reply here.
+ */
+ reply_with_error ("file upload cancelled");
- /* Do NOT send any error. */
return -1;
}
if (r == -2) { /* cancellation from library */
return -1;
}
if (r == -2) { /* cancellation from library */
+ /* This error is ignored by the library since it initiated the
+ * cancel. Nevertheless we must send an error reply here.
+ */
+ reply_with_error ("file upload cancelled");
- /* Do NOT send any error. */
return -1;
}
if (r == -2) { /* cancellation from library */
return -1;
}
if (r == -2) { /* cancellation from library */
+ /* This error is ignored by the library since it initiated the
+ * cancel. Nevertheless we must send an error reply here.
+ */
+ reply_with_error ("file upload cancelled");
- /* Do NOT send any error. */
pr " if (r == -1) {\n";
pr " guestfs___end_busy (g);\n";
trace_return_error ~indent:4 shortname style errcode;
pr " if (r == -1) {\n";
pr " guestfs___end_busy (g);\n";
trace_return_error ~indent:4 shortname style errcode;
+ pr " /* daemon will send an error reply which we discard */\n";
+ pr " guestfs___recv_discard (g, \"%s\");\n" shortname;
pr " return %s;\n" (string_of_errcode errcode);
pr " }\n";
pr " if (r == -2) /* daemon cancelled */\n";
pr " return %s;\n" (string_of_errcode errcode);
pr " }\n";
pr " if (r == -2) /* daemon cancelled */\n";
let mask = Int64.lognot (Int64.pred (Int64.shift_left 1L len)) in
pr " if (optargs_bitmask & UINT64_C(0x%Lx)) {\n" mask;
if is_filein then
let mask = Int64.lognot (Int64.pred (Int64.shift_left 1L len)) in
pr " if (optargs_bitmask & UINT64_C(0x%Lx)) {\n" mask;
if is_filein then
- pr " if (cancel_receive () != -2)\n";
+ pr " cancel_receive ();\n";
pr " reply_with_error (\"unknown option in optional arguments bitmask (this can happen if a program is compiled against a newer version of libguestfs, then run against an older version of the daemon)\");\n";
pr " goto done;\n";
pr " }\n";
pr " reply_with_error (\"unknown option in optional arguments bitmask (this can happen if a program is compiled against a newer version of libguestfs, then run against an older version of the daemon)\");\n";
pr " goto done;\n";
pr " }\n";
pr "\n";
pr " if (!xdr_guestfs_%s_args (xdr_in, &args)) {\n" name;
if is_filein then
pr "\n";
pr " if (!xdr_guestfs_%s_args (xdr_in, &args)) {\n" name;
if is_filein then
- pr " if (cancel_receive () != -2)\n";
- pr " reply_with_error (\"daemon failed to decode procedure arguments\");\n";
+ pr " cancel_receive ();\n";
+ pr " reply_with_error (\"daemon failed to decode procedure arguments\");\n";
pr " goto done;\n";
pr " }\n";
let pr_args n =
pr " goto done;\n";
pr " }\n";
let pr_args n =
pr " sizeof (char *) * (args.%s.%s_len+1));\n" n n;
pr " if (%s == NULL) {\n" n;
if is_filein then
pr " sizeof (char *) * (args.%s.%s_len+1));\n" n n;
pr " if (%s == NULL) {\n" n;
if is_filein then
- pr " if (cancel_receive () != -2)\n";
- pr " reply_with_perror (\"realloc\");\n";
+ pr " cancel_receive ();\n";
+ pr " reply_with_perror (\"realloc\");\n";
pr " goto done;\n";
pr " }\n";
pr " %s[args.%s.%s_len] = NULL;\n" n n n;
pr " goto done;\n";
pr " }\n";
pr " %s[args.%s.%s_len] = NULL;\n" n n n;
rhbz578407.sh \
rhbz580246.sh \
test-add-domain.sh \
rhbz578407.sh \
rhbz580246.sh \
test-add-domain.sh \
+ test-both-ends-cancel.sh \
test-cancellation-download-librarycancels.sh \
test-cancellation-upload-daemoncancels.sh \
test-copy.sh \
test-cancellation-download-librarycancels.sh \
test-cancellation-upload-daemoncancels.sh \
test-copy.sh \
test-bootbootboot.sh
FAILING_TESTS = \
test-bootbootboot.sh
FAILING_TESTS = \
- test-both-ends-cancel.sh \
test-qemudie-launchfail.sh
random_val := $(shell awk 'BEGIN{srand(); print 1+int(255*rand())}' < /dev/null)
test-qemudie-launchfail.sh
random_val := $(shell awk 'BEGIN{srand(); print 1+int(255*rand())}' < /dev/null)
extern int guestfs___end_busy (guestfs_h *g);
extern int guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint, uint64_t optargs_bitmask, xdrproc_t xdrp, char *args);
extern int guestfs___recv (guestfs_h *g, const char *fn, struct guestfs_message_header *hdr, struct guestfs_message_error *err, xdrproc_t xdrp, char *ret);
extern int guestfs___end_busy (guestfs_h *g);
extern int guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint, uint64_t optargs_bitmask, xdrproc_t xdrp, char *args);
extern int guestfs___recv (guestfs_h *g, const char *fn, struct guestfs_message_header *hdr, struct guestfs_message_error *err, xdrproc_t xdrp, char *ret);
+extern int guestfs___recv_discard (guestfs_h *g, const char *fn);
extern int guestfs___send_file (guestfs_h *g, const char *filename);
extern int guestfs___recv_file (guestfs_h *g, const char *filename);
extern int guestfs___send_to_daemon (guestfs_h *g, const void *v_buf, size_t n);
extern int guestfs___send_file (guestfs_h *g, const char *filename);
extern int guestfs___recv_file (guestfs_h *g, const char *filename);
extern int guestfs___send_to_daemon (guestfs_h *g, const void *v_buf, size_t n);
if (fd == -1) {
perrorf (g, "open: %s", filename);
send_file_cancellation (g);
if (fd == -1) {
perrorf (g, "open: %s", filename);
send_file_cancellation (g);
- /* Daemon sees cancellation and won't reply, so caller can
- * just return here.
- */
+/* Same as guestfs___recv, but it discards the reply message. */
+int
+guestfs___recv_discard (guestfs_h *g, const char *fn)
+{
+ void *buf;
+ uint32_t size;
+ int r;
+
+ again:
+ r = guestfs___recv_from_daemon (g, &size, &buf);
+ if (r == -1)
+ return -1;
+
+ /* This can happen if a cancellation happens right at the end
+ * of us sending a FileIn parameter to the daemon. Discard. The
+ * daemon should send us an error message next.
+ */
+ if (size == GUESTFS_CANCEL_FLAG)
+ goto again;
+
+ if (size == GUESTFS_LAUNCH_FLAG) {
+ error (g, "%s: received unexpected launch flag from daemon when expecting reply", fn);
+ return -1;
+ }
+
+ return 0;
+}
+
/* Receive a file. */
/* Returns -1 = error, 0 = EOF, > 0 = more data */
/* Receive a file. */
/* Returns -1 = error, 0 = EOF, > 0 = more data */