Re: [Libguestfs] [nbdkit PATCH 1/2] retry: Add in retry support during .open

Now that a filter can open a backend as many times as it wants, there's no longer a technical reason we can't retry .open. However, adding retry logic here does mean we have to weaken an assert in the server backend code, since prepare can now be reached more than once. Test coverage will be added in a separate patch, so that it becomes easy to swap patch order and see that the test fails without this patch. See also https://bugzilla.redhat.com/show_bug.cgi?id=1841820 --- server/backend.c | 7 +++- filters/retry/retry.c | 98 +++++++++++++++++++++++++------------------ 2 files changed, 63 insertions(+), 42 deletions(-) diff --git a/server/backend.c b/server/backend.c index 3bbba601..45889ea9 100644 --- a/server/backend.c +++ b/server/backend.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2021 Red Hat Inc. + * Copyright (C) 2013-2023 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -312,7 +312,10 @@ backend_prepare (struct context *c) struct backend *b = c->b; assert (c->handle); - assert ((c->state & (HANDLE_OPEN | HANDLE_CONNECTED)) == HANDLE_OPEN); + assert (c->state & HANDLE_OPEN); + + if (c->state & HANDLE_CONNECTED) + return 0;

/* Call these in order starting from the filter closest to the * plugin, similar to typical .open order. But remember that diff --git a/filters/retry/retry.c b/filters/retry/retry.c index fb1d0526..7abf74c4 100644 --- a/filters/retry/retry.c +++ b/filters/retry/retry.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2019-2021 Red Hat Inc. + * Copyright (C) 2019-2023 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -113,45 +113,6 @@ struct retry_handle { bool open; }; -static void * -retry_open (nbdkit_next_open *next, nbdkit_context *nxdata, - int readonly, const char *exportname, int is_tls) -{ - struct retry_handle *h; - - if (next (nxdata, readonly, exportname) == -1) - return NULL; - - h = malloc (sizeof *h); - if (h == NULL) { - nbdkit_error ("malloc: %m"); - return NULL; - } - - h->readonly = readonly; - h->exportname = strdup (exportname); - h->context = nxdata; - if (h->exportname == NULL) { - nbdkit_error ("strdup: %m"); - free (h); - return NULL; - } - h->reopens = 0; - h->open = true; - - return h; -} - -static void -retry_close (void *handle) -{ - struct retry_handle *h = handle; - - nbdkit_debug ("reopens needed: %u", h->reopens); - free (h->exportname); - free (h); -} - /* This function encapsulates the common retry logic used across all * data commands. If it returns true then the data command will retry * the operation. ???struct retry_data??? is stack data saved between @@ -247,6 +208,63 @@ do_retry (struct retry_handle *h, struct retry_data *data, return true; } +static void * +retry_open (nbdkit_next_open *next, nbdkit_context *nxdata, + int readonly, const char *exportname, int is_tls) +{ + struct retry_handle *h; + struct retry_data data = {0}; + + h = malloc (sizeof *h); + if (h == NULL) { + nbdkit_error ("malloc: %m"); + return NULL; + } + + h->readonly = readonly; + h->exportname = strdup (exportname); + h->context = nxdata; + if (h->exportname == NULL) { + nbdkit_error ("strdup: %m"); + free (h); + return NULL; + } + h->reopens = 0; + + if (next (nxdata, readonly, exportname) != -1) + h->open = true; + else { + /* Careful - our .open must not return a handle unless do_retry() + * works, as the caller's next action will be calling .get_size + * and similar probe functions which we do not bother to wire up + * into retry logic because they only need to be used right after + * connecting. + */ + nbdkit_next *next_handle = NULL; + int err = ESHUTDOWN; + + while (! h->open && do_retry (h, &data, &next_handle, "open", &err)) + ; + + if (! h->open) { + free (h->exportname); + free (h); + return NULL; + } + } + return h; +} + +static void +retry_close (void *handle) +{ + struct retry_handle *h = handle; + + nbdkit_debug ("reopens needed: %u", h->reopens); + free (h->exportname); + free (h); +} + static int retry_pread (nbdkit_next *next, void *handle, void *buf, uint32_t count, uint64_t offset, --