Re: [Libguestfs] [PATCH nbdkit v2] curl: Implement header and cookie scripts.

What happens if you have header-script-renew=5, but a very slow client that sits idle more more than 5 seconds at a time? Are you burning CPU time every five seconds just to throw it away, or are you letting it idle until the start of each client command where you check whether the time since last renew is larger than the limit. (I guess that's implementation, not worth documenting, so I'll see if I spot it below)

>+ nbdkit curl https://service.example.com/disk.img \ >+ header-script=' >+ echo -n "Authorization: Bearer " 'echo -n' is not 100% portable; better might be using 'printf'

>+ curl -s -X POST https://auth.example.com/login | >+ jq -r .token >+ ' \ >+ header-script-renew=50 Example is inconsistent with text (45 vs. 50), but it looks like you answered my question above, and DO let the script idle when the client is idle.

>+ SERVER=esx.example.com >+ DCPATH=data >+ DS=datastore1 >+ GUEST=guest-name >+ URL="https://$SERVER/folder/$GUEST/$GUEST-flat.vmdk?dcPath=$DCPATH&dsName=$DS" >+ >+ nbdkit curl "$URL" \ >+ cookie-script=' >+ curl --head -s --insecure -u root:password "$url" | >+ sed -ne '{ s/^Set-Cookie: \([^;]*\);.*/\1/ip }' >+ ' \ Umm, this is nesting '' inside ''. You probably need '\'' in the sed line.

>+ nbdkit curl "$URL" \ >+ header-script=' echo -n "Authorization: Bearer "; /tmp/auth.sh ' \ Another s/echo -n/printf/

>+ header-script-renew=200 \ >+ --filter=gzip >+ >+Note that this exposes a tar file over NBD. See also >+L<nbdkit-tar-filter(1)>. >+ > =head1 DEBUG FLAG >+++ b/plugins/curl/curl.c >@@ -48,9 +48,11 @@ > #include <nbdkit-plugin.h> >-#include "cleanup.h" > #include "ascii-ctype.h" > #include "ascii-string.h" >+#include "cleanup.h" >+ >+#include "curldefs.h" > /* Macro CURL_AT_LEAST_VERSION was added in 2015 (Curl 7.43) so if the > * macro isn't present then Curl is very old. >@@ -61,24 +63,29 @@ > #endif > #endif >-static const char *url = NULL; /* required */ >+/* Plugin configuration. */ >+const char *url = NULL; /* required */ >-static const char *cainfo = NULL; >-static const char *capath = NULL; >-static char *cookie = NULL; >-static struct curl_slist *headers = NULL; >-static char *password = NULL; >-static long protocols = CURLPROTO_ALL; >-static const char *proxy = NULL; >-static char *proxy_password = NULL; >-static const char *proxy_user = NULL; >-static bool sslverify = true; >-static bool tcp_keepalive = false; >-static bool tcp_nodelay = true; >-static uint32_t timeout = 0; >-static const char *unix_socket_path = NULL; >-static const char *user = NULL; >-static const char *user_agent = NULL; >+const char *cainfo = NULL; >+const char *capath = NULL; >+char *cookie = NULL; >+const char *cookie_script = NULL; >+unsigned cookie_script_renew = 0; >+struct curl_slist *headers = NULL; >+const char *header_script = NULL; >+unsigned header_script_renew = 0; >+char *password = NULL; All of these variables are already guaranteed zero-initialized without being explicit, >+long protocols = CURLPROTO_ALL; although consistency with this initialization (which is not necessarily 0) makes sense.

>+ >+/* This is called with the lock held when we must run or re-run the >+ * header-script. >+ */ >+static int >+run_header_script (struct curl_handle *h) >+{ >+ >+/* This is called with the lock held when we must run or re-run the >+ * cookie-script. >+ */ >+static int >+run_cookie_script (struct curl_handle *h) >+{ >+ int fd; These two look similar, is it worth refactoring into a common helper routine?