On Fri, Jan 27, 2023 at 02:41:22PM -0600, Eric Blake wrote:
> 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
> ---
> +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);
Uninit memory...
> + 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 {
and h->open has not been assigned if we get here...
> + /* 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))
...even though I assumed it was zero-initialized here. Will fix.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
