search for: vddk_open

...s *params; /* connection parameters */ VixDiskLibConnection connection; /* connection */ VixDiskLibHandle handle; /* disk handle */ + int readonly; }; static inline VixDiskLibConnectParams * @@ -557,6 +569,7 @@ free_connect_params (VixDiskLibConnectParams *params) static void * vddk_open (int readonly) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); struct vddk_handle *h; VixError err; uint32_t flags; @@ -567,6 +580,7 @@ vddk_open (int readonly) return NULL; } + h->readonly = readonly; h->params = allocate_connect_params (); if (h->pa...

In theory this patch depends on this series: https://www.redhat.com/archives/libguestfs/2020-August/msg00021.html In practice I believe they're independent of each other, but the above series makes it easier to test. Rich.

...Lock protecting open/close calls - see above. */ > +static pthread_mutex_t open_close_lock = PTHREAD_MUTEX_INITIALIZER; > > /* The per-connection handle. */ > struct vddk_handle { > @@ -524,6 +531,7 @@ free_connect_params (VixDiskLibConnectParams *params) > static void * > vddk_open (int readonly) > { > + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); > struct vddk_handle *h; > VixError err; > uint32_t flags; > @@ -616,6 +624,7 @@ vddk_open (int readonly) > static void > vddk_close (void *handle) > { > + ACQUIRE_LOCK_FOR_CUR...

I believe this roughly implements Nir's proposal here: https://www.redhat.com/archives/libguestfs/2020-August/msg00028.html Unfortunately it doesn't work for me. It actually slows things down quite a lot, for reasons I don't understand. Note the adjustment of the pool-max parameter and how it affects the total time. The results are quite reproducible. $ ./nbdkit -r -U - vddk

...matter if the library initializes all fields of the struct, but it is safer to zero the memory. 2. We create the handlers in request threads, so most likely not in the same thread. This may be ok, but risky. I moved the creation of all connections and handles to populate_pool(), called from vddk_open(). This may create more connections and handles than needed if the client does not use multiple inflight requests, but this is not important now. The code compiles but I cannot test it. Does it make any difference? Nir > > Rich. > > -- > Richard Jones, Virtualization Group, Red H...

...AD_MODEL_SERIALIZE_REQUESTS + +/* Lock protecting open/close calls - see above. */ +static pthread_mutex_t open_close_lock = PTHREAD_MUTEX_INITIALIZER; /* The per-connection handle. */ struct vddk_handle { @@ -524,6 +531,7 @@ free_connect_params (VixDiskLibConnectParams *params) static void * vddk_open (int readonly) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); struct vddk_handle *h; VixError err; uint32_t flags; @@ -616,6 +624,7 @@ vddk_open (int readonly) static void vddk_close (void *handle) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); struct vddk_...

...ectParams *params; /* connection parameters */ VixDiskLibConnection connection; /* connection */ VixDiskLibHandle handle; /* disk handle */ + VixDiskLibHandle handle_single_link; /* disk handle for single_link=true */ }; static inline VixDiskLibConnectParams * @@ -522,8 +523,6 @@ vddk_open (int readonly) flags = 0; if (readonly) flags |= VIXDISKLIB_FLAG_OPEN_READ_ONLY; - if (single_link) - flags |= VIXDISKLIB_FLAG_OPEN_SINGLE_LINK; if (unbuffered) flags |= VIXDISKLIB_FLAG_OPEN_UNBUFFERED; @@ -538,6 +537,21 @@ vddk_open (int readonly) nbdkit_debug ("tr...

...> + */ > + pthread_mutex_t vddk_handles_lock; > + pthread_cond_t vddk_handles_cond; > + vddk_handles vddk_handles; > }; > > static inline VixDiskLibConnectParams * > @@ -531,17 +556,28 @@ free_connect_params (VixDiskLibConnectParams *params) > static void * > vddk_open (int readonly) > { > - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); > - struct vddk_handle *h; > + struct handle *h; > VixError err; > - uint32_t flags; > > - h = malloc (sizeof *h); > + h = calloc (1, sizeof *h); > if (h == NULL) { > - nbd...

...ic void @@ -901,6 +901,7 @@ static struct nbdkit_plugin plugin = { .config = vddk_config, .config_complete = vddk_config_complete, .config_help = vddk_config_help, + .magic_config_key = "file", .dump_plugin = vddk_dump_plugin, .open = vddk_open, .close = vddk_close, diff --git a/tests/test-vddk.sh b/tests/test-vddk.sh index d99ebf88..6933f716 100755 --- a/tests/test-vddk.sh +++ b/tests/test-vddk.sh @@ -48,8 +48,7 @@ grep ^vddk_default_libdir= test-vddk.out # a load that we know will fail, but the important part is that dlo...

...LE_FOR_CURRENT_SCOPE macro to get a free handle. + */ + pthread_mutex_t vddk_handles_lock; + pthread_cond_t vddk_handles_cond; + vddk_handles vddk_handles; }; static inline VixDiskLibConnectParams * @@ -531,17 +556,28 @@ free_connect_params (VixDiskLibConnectParams *params) static void * vddk_open (int readonly) { - ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&open_close_lock); - struct vddk_handle *h; + struct handle *h; VixError err; - uint32_t flags; - h = malloc (sizeof *h); + h = calloc (1, sizeof *h); if (h == NULL) { - nbdkit_error ("malloc: %m"); + nbdkit_error...

...mx_spec = value; } + else if (strcmp (key, "single-link") == 0) { + int r = nbdkit_parse_bool (value); + + if (r == -1) + return -1; + single_link = r; + } else { nbdkit_error ("unknown parameter '%s'", key); return -1; @@ -464,6 +472,8 @@ vddk_open (int readonly) flags = 0; if (readonly) flags |= VIXDISKLIB_FLAG_OPEN_READ_ONLY; + if (single_link) + flags |= VIXDISKLIB_FLAG_OPEN_SINGLE_LINK; DEBUG_CALL ("VixDiskLib_Open", "connection, %s, %d, &handle", filename, flags); -- 2.20.1

Couple of fixes to return correct data and one nice-to-have clean-up which is not needed. I just find it nicer to read. Martin Kletzander (3): vddk: Use a separate handle for single-link=true vddk: Do not report hole extents to be zero with single-link=true vddk: Eliminate one needless goto plugins/vddk/vddk.c | 48 +++++++++++++++++++++++++++++++++------------ 1 file changed, 36

We had a query yesterday about the VDDK plugin and making it actually obey the weird "Multithreading Considerations" rules in the VDDK documentation (https://vdc-download.vmware.com/vmwb-repository/dcr-public/8f96698a-0e7b-4d67-bb6c-d18a1d101540/ef536a47-27cd-481a-90ef-76b38e75353c/vsphere-vddk-671-programming-guide.pdf) This patch is my attempt to implement this. The idea is that the

I suppose we may as well implement the only other flag too ... It's not clear what this does, something like O_DIRECT I imagine. Rich.

...)." - static void vddk_dump_plugin (void) { @@ -959,6 +974,7 @@ static struct nbdkit_plugin plugin = { .magic_config_key = "file", .dump_plugin = vddk_dump_plugin, .get_ready = vddk_get_ready, + .after_fork = vddk_after_fork, .open = vddk_open, .close = vddk_close, .get_size = vddk_get_size, -- 2.25.0

...(key, "single-link") == 0) { >+ int r = nbdkit_parse_bool (value); >+ >+ if (r == -1) >+ return -1; >+ single_link = r; >+ } > else { > nbdkit_error ("unknown parameter '%s'", key); > return -1; >@@ -464,6 +472,8 @@ vddk_open (int readonly) > flags = 0; > if (readonly) > flags |= VIXDISKLIB_FLAG_OPEN_READ_ONLY; >+ if (single_link) >+ flags |= VIXDISKLIB_FLAG_OPEN_SINGLE_LINK; > > DEBUG_CALL ("VixDiskLib_Open", > "connection, %s, %d, &handle", fi...

...atic struct nbdkit_plugin plugin = { > .config = vddk_config, > .config_complete = vddk_config_complete, > .config_help = vddk_config_help, > + .magic_config_key = "file", > .dump_plugin = vddk_dump_plugin, > .open = vddk_open, > .close = vddk_close, > diff --git a/tests/test-vddk.sh b/tests/test-vddk.sh > index d99ebf88..6933f716 100755 > --- a/tests/test-vddk.sh > +++ b/tests/test-vddk.sh > @@ -48,8 +48,7 @@ grep ^vddk_default_libdir= test-vddk.out > # a load that we know will fail,...

This is my second attempt at this. The first version (also not working) was here: https://www.redhat.com/archives/libguestfs/2019-October/msg00062.html In part 1/2 I introduce a new .ready_to_serve plugin method which is called after forking and just before accepting any client connection. The idea would be that plugins could start background threads here. However this doesn't work well in

...in = { > > .config = vddk_config, > > .config_complete = vddk_config_complete, > > .config_help = vddk_config_help, > > + .magic_config_key = "file", > > .dump_plugin = vddk_dump_plugin, > > .open = vddk_open, > > .close = vddk_close, > > diff --git a/tests/test-vddk.sh b/tests/test-vddk.sh > > index d99ebf88..6933f716 100755 > > --- a/tests/test-vddk.sh > > +++ b/tests/test-vddk.sh > > @@ -48,8 +48,7 @@ grep ^vddk_default_libdir= test-vddk.out > &gt...

I like this change. I think we were overloading the config_complete method before to do two different things (complete configuration; do any allocation/housekeeping necessary before we can start serving). The only questions in my mind are whether we want this before 1.18, and whether the name ("get_ready") is a good one. Rich.