From: "Richard W.M. Jones" <rjones(a)redhat.com&gt; Originally this state was intended so that in some way you could find out if the appliance was running a command. However there was never a thread-safe way to access the state of the handle, so in effect you could never do anything useful safely with this information. This commit completely removes the BUSY state. The only visible change is to the guestfs_is_busy API. Previously you could never call this safely from another thread. If you called it from the same thread it would always return false (since the current thread can't be running a libguestfs command at that point by definition). Now it always returns false. --- generator/generator_actions.ml | 5 ++- generator/generator_c.ml | 28 ++++----------- perl/t/006-pod-coverage.t | 9 +++-- src/guestfs-internal.h | 4 +-- src/guestfs.pod | 33 +++++++++-------- src/launch.c | 3 +- src/proto.c | 77 +++++++--------------------------------- 7 files changed, 46 insertions(+), 113 deletions(-) diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml index 7c1008b..ace46cb 100644 --- a/generator/generator_actions.ml +++ b/generator/generator_actions.ml @@ -354,13 +354,12 @@ This returns true iff this handle is launching the subprocess For more information on states, see L<guestfs(3)>."); - ("is_busy", (RBool "busy", [], []), -1, [], + ("is_busy", (RBool "busy", [], []), -1, [NotInDocs], [InitNone, Always, TestOutputFalse ( [["is_busy"]])], "is busy processing a command", "\ -This returns true iff this handle is busy processing a command -(in the C<BUSY> state). +This always returns false. Do not use this function. For more information on states, see L<guestfs(3)>."); diff --git a/generator/generator_c.ml b/generator/generator_c.ml index d987c4a..3e7f314 100644 --- a/generator/generator_c.ml +++ b/generator/generator_c.ml @@ -706,17 +706,13 @@ check_reply_header (guestfs_h *g, return 0; } -/* Check we are in the right state to run a high-level action. */ +/* Check the appliance is up when running a daemon_function. */ static int -check_state (guestfs_h *g, const char *caller) +check_appliance_up (guestfs_h *g, const char *caller) { - if (!guestfs__is_ready (g)) { - if (guestfs__is_config (g) || guestfs__is_launching (g)) - error (g, \"%%s: call launch before using this function\\n(in guestfish, don't forget to use the 'run' command)\", - caller); - else - error (g, \"%%s called from the wrong state, %%d != READY\", - caller, guestfs__get_state (g)); + if (guestfs__is_config (g) || guestfs__is_launching (g)) { + error (g, \"%%s: call launch before using this function\\n(in guestfish, don't forget to use the 'run' command)\", + caller); return -1; } return 0; @@ -1166,12 +1162,11 @@ trace_send_line (guestfs_h *g) | _ -> () ) args; - (* Check we are in the right state for sending a request. *) - pr " if (check_state (g, \"%s\") == -1) {\n" shortname; + (* This is a daemon_function so check the appliance is up. *) + pr " if (check_appliance_up (g, \"%s\") == -1) {\n" shortname; trace_return_error ~indent:4 shortname style errcode; pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; - pr " guestfs___set_busy (g);\n"; pr "\n"; (* Send the main header and arguments. *) @@ -1202,7 +1197,6 @@ trace_send_line (guestfs_h *g) trace_return_error ~indent:4 shortname style errcode; pr " error (g, \"%%s: size of input buffer too large\", \"%s\");\n" shortname; - pr " guestfs___end_busy (g);\n"; pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr " args.%s.%s_val = (char *) %s;\n" n n n; @@ -1239,7 +1233,6 @@ trace_send_line (guestfs_h *g) name; ); pr " if (serial == -1) {\n"; - pr " guestfs___end_busy (g);\n"; trace_return_error ~indent:4 shortname style errcode; pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; @@ -1252,7 +1245,6 @@ trace_send_line (guestfs_h *g) | FileIn n -> pr " r = guestfs___send_file (g, %s);\n" n; 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; @@ -1279,7 +1271,6 @@ trace_send_line (guestfs_h *g) pr ");\n"; pr " if (r == -1) {\n"; - pr " guestfs___end_busy (g);\n"; trace_return_error ~indent:4 shortname style errcode; pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; @@ -1287,7 +1278,6 @@ trace_send_line (guestfs_h *g) pr " if (check_reply_header (g, &hdr, GUESTFS_PROC_%s, serial) == -1) {\n" (String.uppercase shortname); - pr " guestfs___end_busy (g);\n"; trace_return_error ~indent:4 shortname style errcode; pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; @@ -1307,7 +1297,6 @@ trace_send_line (guestfs_h *g) pr " err.error_message);\n"; pr " free (err.error_message);\n"; pr " free (err.errno_string);\n"; - pr " guestfs___end_busy (g);\n"; pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr "\n"; @@ -1317,7 +1306,6 @@ trace_send_line (guestfs_h *g) function | FileOut n -> pr " if (guestfs___recv_file (g, %s) == -1) {\n" n; - pr " guestfs___end_busy (g);\n"; trace_return_error ~indent:4 shortname style errcode; pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; @@ -1325,8 +1313,6 @@ trace_send_line (guestfs_h *g) | _ -> () ) args; - pr " guestfs___end_busy (g);\n"; - (match ret with | RErr -> pr " ret_v = 0;\n" diff --git a/perl/t/006-pod-coverage.t b/perl/t/006-pod-coverage.t index 65bb4c2..5227275 100644 --- a/perl/t/006-pod-coverage.t +++ b/perl/t/006-pod-coverage.t @@ -22,7 +22,10 @@ use warnings; eval "use Test::Pod::Coverage 1.00"; plan skip_all => "Test::Pod::Coverage 1.00 required for testing POD" if $@; all_pod_coverage_ok ({ - also_private => [ qr/^test0.*/, - qr/^debug.*/, - qr/^internal.*/ ] + also_private => [ + qr/^debug.*/, + qr/^is_busy$/, + qr/^internal.*/, + qr/^test0.*/, + ] }); diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 0492c9e..a41212d 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -142,7 +142,7 @@ #define ROUTER "169.254.2.2" /* GuestFS handle and connection. */ -enum state { CONFIG, LAUNCHING, READY, BUSY, NO_HANDLE }; +enum state { CONFIG, LAUNCHING, READY, NO_HANDLE }; /* Attach method. */ enum attach_method { ATTACH_METHOD_APPLIANCE = 0, ATTACH_METHOD_UNIX }; @@ -406,8 +406,6 @@ extern void guestfs___free_fuse (guestfs_h *g); #endif extern void guestfs___free_inspect_info (guestfs_h *g); extern void guestfs___free_drives (struct drive **drives); -extern int guestfs___set_busy (guestfs_h *g); -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); diff --git a/src/guestfs.pod b/src/guestfs.pod index 3f5410a..d22fa64 100644 --- a/src/guestfs.pod +++ b/src/guestfs.pod @@ -2400,23 +2400,23 @@ libguestfs uses a state machine to model the child process: / \ | CONFIG | \__________/ - ^ ^ ^ \ - / | \ \ guestfs_launch - / | _\__V______ - / | / \ - / | | LAUNCHING | - / | \___________/ - / | / - / | guestfs_launch - / | / - ______ / __|____V - / \ ------> / \ - | BUSY | | READY | - \______/ <------ \________/ + ^ ^ \ + | \ \ guestfs_launch + | _\__V______ + | / \ + | | LAUNCHING | + | \___________/ + | / + | guestfs_launch + | / + __|____V + / \ + | READY | + \________/ The normal transitions are (1) CONFIG (when the handle is created, but there is no child process), (2) LAUNCHING (when the child process is -booting up), (3) alternating between READY and BUSY as commands are +booting up), (3) READY meaning the appliance is up, actions can be issued to, and carried out by, the child process. The guest may be killed by L</guestfs_kill_subprocess>, or may die @@ -2434,9 +2434,8 @@ while it is running. API actions such as L</guestfs_mount> can only be issued when in the READY state. These API calls block waiting for the command to be -carried out (ie. the state to transition to BUSY and then back to -READY). There are no non-blocking versions, and no way to issue more -than one command per handle at the same time. +carried out. There are no non-blocking versions, and no way to issue +more than one command per handle at the same time. Finally, the child process sends asynchronous messages back to the main program, such as kernel log messages. You can register a diff --git a/src/launch.c b/src/launch.c index b61c538..c66cbac 100644 --- a/src/launch.c +++ b/src/launch.c @@ -1456,7 +1456,8 @@ guestfs__is_ready (guestfs_h *g) int guestfs__is_busy (guestfs_h *g) { - return g->state == BUSY; + /* There used to be a BUSY state but it was removed in 1.17.36. */ + return 0; } int diff --git a/src/proto.c b/src/proto.c index 16cff4f..95d619f 100644 --- a/src/proto.c +++ b/src/proto.c @@ -84,30 +84,24 @@ * (2) A simple RPC (eg. "mount"). We write the request, then read * the reply. The sequence of calls is: * - * guestfs___set_busy * guestfs___send * guestfs___recv - * guestfs___end_busy * * (3) An RPC with FileOut parameters (eg. "upload"). We write the * request, then write the file(s), then read the reply. The sequence * of calls is: * - * guestfs___set_busy * guestfs___send * guestfs___send_file (possibly multiple times) * guestfs___recv - * guestfs___end_busy * * (4) An RPC with FileIn parameters (eg. "download"). We write the * request, then read the reply, then read the file(s). The sequence * of calls is: * - * guestfs___set_busy * guestfs___send * guestfs___recv * guestfs___recv_file (possibly multiple times) - * guestfs___end_busy * * (5) Both FileOut and FileIn parameters. There are no calls like * this in the current API, but they would be implemented as a @@ -181,39 +175,6 @@ message_summary (const void *buf, size_t n, char *workspace) return workspace; } -int -guestfs___set_busy (guestfs_h *g) -{ - if (g->state != READY) { - error (g, _("guestfs_set_busy: called when in state %d != READY"), - g->state); - return -1; - } - g->state = BUSY; - return 0; -} - -int -guestfs___end_busy (guestfs_h *g) -{ - switch (g->state) - { - case BUSY: - g->state = READY; - break; - case CONFIG: - case READY: - break; - - case LAUNCHING: - case NO_HANDLE: - default: - error (g, _("guestfs_end_busy: called when in state %d"), g->state); - return -1; - } - return 0; -} - /* This is called if we detect EOF, ie. qemu died. */ static void child_cleanup (guestfs_h *g) @@ -395,6 +356,7 @@ check_for_daemon_cancellation_or_eof (guestfs_h *g, int fd) /* Read and process progress messages that happen during FileIn. */ if (flag == GUESTFS_PROGRESS_FLAG) { char buf[PROGRESS_MESSAGE_SIZE]; + guestfs_progress message; n = really_read_from_socket (g, fd, buf, PROGRESS_MESSAGE_SIZE); if (n == -1) @@ -404,15 +366,11 @@ check_for_daemon_cancellation_or_eof (guestfs_h *g, int fd) return -1; } - if (g->state == BUSY) { - guestfs_progress message; - - xdrmem_create (&xdr, buf, PROGRESS_MESSAGE_SIZE, XDR_DECODE); - xdr_guestfs_progress (&xdr, &message); - xdr_destroy (&xdr); + xdrmem_create (&xdr, buf, PROGRESS_MESSAGE_SIZE, XDR_DECODE); + xdr_guestfs_progress (&xdr, &message); + xdr_destroy (&xdr); - guestfs___progress_message_callback (g, &message); - } + guestfs___progress_message_callback (g, &message); return 0; } @@ -713,15 +671,14 @@ guestfs___recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void **buf_rtn) #endif if (*size_rtn == GUESTFS_PROGRESS_FLAG) { - if (g->state == BUSY) { - guestfs_progress message; - XDR xdr; - xdrmem_create (&xdr, *buf_rtn, PROGRESS_MESSAGE_SIZE, XDR_DECODE); - xdr_guestfs_progress (&xdr, &message); - xdr_destroy (&xdr); + guestfs_progress message; + XDR xdr; - guestfs___progress_message_callback (g, &message); - } + xdrmem_create (&xdr, *buf_rtn, PROGRESS_MESSAGE_SIZE, XDR_DECODE); + xdr_guestfs_progress (&xdr, &message); + xdr_destroy (&xdr); + + guestfs___progress_message_callback (g, &message); free (*buf_rtn); *buf_rtn = NULL; @@ -806,11 +763,6 @@ guestfs___send (guestfs_h *g, int proc_nr, char *msg_out; size_t msg_out_size; - if (g->state != BUSY) { - error (g, _("guestfs___send: state %d != BUSY"), g->state); - return -1; - } - /* We have to allocate this message buffer on the heap because * it is quite large (although will be mostly unused). We * can't allocate it on the stack because in some environments @@ -987,11 +939,6 @@ send_file_chunk (guestfs_h *g, int cancel, const char *buf, size_t buflen) char *msg_out; size_t msg_out_size; - if (g->state != BUSY) { - error (g, _("send_file_chunk: state %d != READY"), g->state); - return -1; - } - /* Allocate the chunk buffer. Don't use the stack to avoid * excessive stack usage and unnecessary copies. */ -- 1.7.10