| Message ID | 02a82c80a35ab60b98028c85aa94f688a2843943.1610638428.git.jag.raman@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Initial support for multi-process Qemu | expand |
On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote: > +int qio_channel_readv_full_all(QIOChannel *ioc, > + const struct iovec *iov, > + size_t niov, > + int **fds, size_t *nfds, > + Error **errp) > { > - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp); > + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp); > > if (ret == 0) { > - ret = -1; > error_setg(errp, > "Unexpected end-of-file before all bytes were read"); qio_channel_readv_full_all_eof() can read file descriptors but no data and return 0. Here that case is converted into an error and the file descriptors aren't closed, freed, and fds/nfds isn't cleared.
> On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote: >> +int qio_channel_readv_full_all(QIOChannel *ioc, >> + const struct iovec *iov, >> + size_t niov, >> + int **fds, size_t *nfds, >> + Error **errp) >> { >> - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp); >> + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp); >> >> if (ret == 0) { >> - ret = -1; >> error_setg(errp, >> "Unexpected end-of-file before all bytes were read"); > > qio_channel_readv_full_all_eof() can read file descriptors but no data > and return 0. > > Here that case is converted into an error and the file descriptors > aren't closed, freed, and fds/nfds isn't cleared. That’s a valid point. I’m wondering if the fix for this case should be in qio_channel_readv_full_all_eof(), instead of here. qio_channel_readv_full_all_eof() should probably return error (-1) if the amount of data read does not match iov_size(). If the caller is only expecting to read fds, and not any data, it would indicate that by setting iov to NULL and/or setting niov=0. If the caller is setting these parameters, it means it is expecting data.Does that sound good? Thanks!
On Thu, Jan 14, 2021 at 12:55:58PM -0500, Jag Raman wrote: > > > > On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > > > On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote: > >> +int qio_channel_readv_full_all(QIOChannel *ioc, > >> + const struct iovec *iov, > >> + size_t niov, > >> + int **fds, size_t *nfds, > >> + Error **errp) > >> { > >> - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp); > >> + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp); > >> > >> if (ret == 0) { > >> - ret = -1; > >> error_setg(errp, > >> "Unexpected end-of-file before all bytes were read"); > > > > qio_channel_readv_full_all_eof() can read file descriptors but no data > > and return 0. > > > > Here that case is converted into an error and the file descriptors > > aren't closed, freed, and fds/nfds isn't cleared. > > That’s a valid point. I’m wondering if the fix for this case should be in > qio_channel_readv_full_all_eof(), instead of here. > > qio_channel_readv_full_all_eof() should probably return error (-1) if the > amount of data read does not match iov_size(). If the caller is only expecting > to read fds, and not any data, it would indicate that by setting iov to NULL > and/or setting niov=0. If the caller is setting these parameters, it means it is > expecting data.Does that sound good? The API spec for the existing _eof() methods says: * The function will wait for all requested data * to be read, yielding from the current coroutine * if required. * * If end-of-file occurs before any data is read, * no error is reported; otherwise, if it occurs * before all requested data has been read, an error * will be reported. IOW, return '0' is *only* valid if we've not read anything. I consider file descriptors to be something. IOW, qio_channel_readv_full_all_eof must only return 0, if it didn't read any data and also didn't receive any file descriptors. So yeah, we must return -1 in the scenario Stefan describes Regards, Daniel
> On Jan 14, 2021, at 1:00 PM, Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Thu, Jan 14, 2021 at 12:55:58PM -0500, Jag Raman wrote: >> >> >>> On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: >>> >>> On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote: >>>> +int qio_channel_readv_full_all(QIOChannel *ioc, >>>> + const struct iovec *iov, >>>> + size_t niov, >>>> + int **fds, size_t *nfds, >>>> + Error **errp) >>>> { >>>> - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp); >>>> + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp); >>>> >>>> if (ret == 0) { >>>> - ret = -1; >>>> error_setg(errp, >>>> "Unexpected end-of-file before all bytes were read"); >>> >>> qio_channel_readv_full_all_eof() can read file descriptors but no data >>> and return 0. >>> >>> Here that case is converted into an error and the file descriptors >>> aren't closed, freed, and fds/nfds isn't cleared. >> >> That’s a valid point. I’m wondering if the fix for this case should be in >> qio_channel_readv_full_all_eof(), instead of here. >> >> qio_channel_readv_full_all_eof() should probably return error (-1) if the >> amount of data read does not match iov_size(). If the caller is only expecting >> to read fds, and not any data, it would indicate that by setting iov to NULL >> and/or setting niov=0. If the caller is setting these parameters, it means it is >> expecting data.Does that sound good? > > The API spec for the existing _eof() methods says: > > * The function will wait for all requested data > * to be read, yielding from the current coroutine > * if required. > * > * If end-of-file occurs before any data is read, > * no error is reported; otherwise, if it occurs > * before all requested data has been read, an error > * will be reported. > > > IOW, return '0' is *only* valid if we've not read anything. I consider > file descriptors to be something. > > IOW, qio_channel_readv_full_all_eof must only return 0, if it didn't > read any data and also didn't receive any file descriptors. So yeah, > we must return -1 in the scenario Stefan describes That makes sense to me. Reading “fds" is something, which is different from our previous understanding. I thought data only meant iov, and not fds. So the return values for qio_channel_readv_full_all_eof() would be: - ‘0’ only if EOF is reached without reading any fds and data. - ‘1’ if all data that the caller expects are read (even if the caller reads fds exclusively, without any iovs) - ‘-1’ otherwise, considered as error qio_channel_readv_full_all() would return: - ‘0’ if all the data that caller expects are read - ‘-1’ otherwise, considered as error Hey Stefan, Does this sound good to you? Thank you! -- Jag > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| >
On Thu, Jan 14, 2021 at 01:24:37PM -0500, Jag Raman wrote: > > > > On Jan 14, 2021, at 1:00 PM, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > On Thu, Jan 14, 2021 at 12:55:58PM -0500, Jag Raman wrote: > >> > >> > >>> On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > >>> > >>> On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote: > >>>> +int qio_channel_readv_full_all(QIOChannel *ioc, > >>>> + const struct iovec *iov, > >>>> + size_t niov, > >>>> + int **fds, size_t *nfds, > >>>> + Error **errp) > >>>> { > >>>> - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp); > >>>> + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp); > >>>> > >>>> if (ret == 0) { > >>>> - ret = -1; > >>>> error_setg(errp, > >>>> "Unexpected end-of-file before all bytes were read"); > >>> > >>> qio_channel_readv_full_all_eof() can read file descriptors but no data > >>> and return 0. > >>> > >>> Here that case is converted into an error and the file descriptors > >>> aren't closed, freed, and fds/nfds isn't cleared. > >> > >> That’s a valid point. I’m wondering if the fix for this case should be in > >> qio_channel_readv_full_all_eof(), instead of here. > >> > >> qio_channel_readv_full_all_eof() should probably return error (-1) if the > >> amount of data read does not match iov_size(). If the caller is only expecting > >> to read fds, and not any data, it would indicate that by setting iov to NULL > >> and/or setting niov=0. If the caller is setting these parameters, it means it is > >> expecting data.Does that sound good? > > > > The API spec for the existing _eof() methods says: > > > > * The function will wait for all requested data > > * to be read, yielding from the current coroutine > > * if required. > > * > > * If end-of-file occurs before any data is read, > > * no error is reported; otherwise, if it occurs > > * before all requested data has been read, an error > > * will be reported. > > > > > > IOW, return '0' is *only* valid if we've not read anything. I consider > > file descriptors to be something. > > > > IOW, qio_channel_readv_full_all_eof must only return 0, if it didn't > > read any data and also didn't receive any file descriptors. So yeah, > > we must return -1 in the scenario Stefan describes > > That makes sense to me. Reading “fds" is something, which is different > from our previous understanding. I thought data only meant iov, and not fds. > > So the return values for qio_channel_readv_full_all_eof() would be: > - ‘0’ only if EOF is reached without reading any fds and data. > - ‘1’ if all data that the caller expects are read (even if the caller reads > fds exclusively, without any iovs) > - ‘-1’ otherwise, considered as error > > qio_channel_readv_full_all() would return: > - ‘0’ if all the data that caller expects are read > - ‘-1’ otherwise, considered as error > > Hey Stefan, > > Does this sound good to you? The while (nlocal_iov > 0) loop only runs if the caller has requested to read at least some data, so the fds-only case doesn't work yet. This suggests that no current QEMU code relies on the fds-only case. Therefore you could change the doc comment to clarify this instead of adding support for this case to the code. But if you would to fully support the fds-only case that would be even better. Stefan
> On Jan 15, 2021, at 4:20 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > On Thu, Jan 14, 2021 at 01:24:37PM -0500, Jag Raman wrote: >> >> >>> On Jan 14, 2021, at 1:00 PM, Daniel P. Berrangé <berrange@redhat.com> wrote: >>> >>> On Thu, Jan 14, 2021 at 12:55:58PM -0500, Jag Raman wrote: >>>> >>>> >>>>> On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: >>>>> >>>>> On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote: >>>>>> +int qio_channel_readv_full_all(QIOChannel *ioc, >>>>>> + const struct iovec *iov, >>>>>> + size_t niov, >>>>>> + int **fds, size_t *nfds, >>>>>> + Error **errp) >>>>>> { >>>>>> - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp); >>>>>> + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp); >>>>>> >>>>>> if (ret == 0) { >>>>>> - ret = -1; >>>>>> error_setg(errp, >>>>>> "Unexpected end-of-file before all bytes were read"); >>>>> >>>>> qio_channel_readv_full_all_eof() can read file descriptors but no data >>>>> and return 0. >>>>> >>>>> Here that case is converted into an error and the file descriptors >>>>> aren't closed, freed, and fds/nfds isn't cleared. >>>> >>>> That’s a valid point. I’m wondering if the fix for this case should be in >>>> qio_channel_readv_full_all_eof(), instead of here. >>>> >>>> qio_channel_readv_full_all_eof() should probably return error (-1) if the >>>> amount of data read does not match iov_size(). If the caller is only expecting >>>> to read fds, and not any data, it would indicate that by setting iov to NULL >>>> and/or setting niov=0. If the caller is setting these parameters, it means it is >>>> expecting data.Does that sound good? >>> >>> The API spec for the existing _eof() methods says: >>> >>> * The function will wait for all requested data >>> * to be read, yielding from the current coroutine >>> * if required. >>> * >>> * If end-of-file occurs before any data is read, >>> * no error is reported; otherwise, if it occurs >>> * before all requested data has been read, an error >>> * will be reported. >>> >>> >>> IOW, return '0' is *only* valid if we've not read anything. I consider >>> file descriptors to be something. >>> >>> IOW, qio_channel_readv_full_all_eof must only return 0, if it didn't >>> read any data and also didn't receive any file descriptors. So yeah, >>> we must return -1 in the scenario Stefan describes >> >> That makes sense to me. Reading “fds" is something, which is different >> from our previous understanding. I thought data only meant iov, and not fds. >> >> So the return values for qio_channel_readv_full_all_eof() would be: >> - ‘0’ only if EOF is reached without reading any fds and data. >> - ‘1’ if all data that the caller expects are read (even if the caller reads >> fds exclusively, without any iovs) >> - ‘-1’ otherwise, considered as error >> >> qio_channel_readv_full_all() would return: >> - ‘0’ if all the data that caller expects are read >> - ‘-1’ otherwise, considered as error >> >> Hey Stefan, >> >> Does this sound good to you? > > The while (nlocal_iov > 0) loop only runs if the caller has requested to > read at least some data, so the fds-only case doesn't work yet. > > This suggests that no current QEMU code relies on the fds-only case. > Therefore you could change the doc comment to clarify this instead of > adding support for this case to the code. > > But if you would to fully support the fds-only case that would be even > better. Thanks for confirming, Stefan! Since we are adding fds support in this patch, I suppose we could add the fds-only case since you feel that’s a better approach. I believe it’s reasonable to assume that the caller wants to read file descriptors if it sets the fds double-pointer. I think it would be reasonable to wait for data if the caller expects to read either data or fds, as follows: while ((nlocal_iov > 0) || local_fds) Thank you! > > Stefan
> On Jan 15, 2021, at 4:20 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > On Thu, Jan 14, 2021 at 01:24:37PM -0500, Jag Raman wrote: >> >> >>> On Jan 14, 2021, at 1:00 PM, Daniel P. Berrangé <berrange@redhat.com> wrote: >>> >>> On Thu, Jan 14, 2021 at 12:55:58PM -0500, Jag Raman wrote: >>>> >>>> >>>>> On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: >>>>> >>>>> On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote: >>>>>> +int qio_channel_readv_full_all(QIOChannel *ioc, >>>>>> + const struct iovec *iov, >>>>>> + size_t niov, >>>>>> + int **fds, size_t *nfds, >>>>>> + Error **errp) >>>>>> { >>>>>> - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp); >>>>>> + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp); >>>>>> >>>>>> if (ret == 0) { >>>>>> - ret = -1; >>>>>> error_setg(errp, >>>>>> "Unexpected end-of-file before all bytes were read"); >>>>> >>>>> qio_channel_readv_full_all_eof() can read file descriptors but no data >>>>> and return 0. >>>>> >>>>> Here that case is converted into an error and the file descriptors >>>>> aren't closed, freed, and fds/nfds isn't cleared. >>>> >>>> That’s a valid point. I’m wondering if the fix for this case should be in >>>> qio_channel_readv_full_all_eof(), instead of here. >>>> >>>> qio_channel_readv_full_all_eof() should probably return error (-1) if the >>>> amount of data read does not match iov_size(). If the caller is only expecting >>>> to read fds, and not any data, it would indicate that by setting iov to NULL >>>> and/or setting niov=0. If the caller is setting these parameters, it means it is >>>> expecting data.Does that sound good? >>> >>> The API spec for the existing _eof() methods says: >>> >>> * The function will wait for all requested data >>> * to be read, yielding from the current coroutine >>> * if required. >>> * >>> * If end-of-file occurs before any data is read, >>> * no error is reported; otherwise, if it occurs >>> * before all requested data has been read, an error >>> * will be reported. >>> >>> >>> IOW, return '0' is *only* valid if we've not read anything. I consider >>> file descriptors to be something. >>> >>> IOW, qio_channel_readv_full_all_eof must only return 0, if it didn't >>> read any data and also didn't receive any file descriptors. So yeah, >>> we must return -1 in the scenario Stefan describes >> >> That makes sense to me. Reading “fds" is something, which is different >> from our previous understanding. I thought data only meant iov, and not fds. >> >> So the return values for qio_channel_readv_full_all_eof() would be: >> - ‘0’ only if EOF is reached without reading any fds and data. >> - ‘1’ if all data that the caller expects are read (even if the caller reads >> fds exclusively, without any iovs) >> - ‘-1’ otherwise, considered as error >> >> qio_channel_readv_full_all() would return: >> - ‘0’ if all the data that caller expects are read >> - ‘-1’ otherwise, considered as error >> >> Hey Stefan, >> >> Does this sound good to you? > > The while (nlocal_iov > 0) loop only runs if the caller has requested to > read at least some data, so the fds-only case doesn't work yet. > > This suggests that no current QEMU code relies on the fds-only case. > Therefore you could change the doc comment to clarify this instead of > adding support for this case to the code. > > But if you would to fully support the fds-only case that would be even > better. > > Stefan We are working on sending the next revision out. We could handle the fds-only case by altering the while loop condition to be: ((nlocal_iov > 0) || local_fds) For reference, we would need to handle the following cases: len < 0; !partial, !*nfds => ret = -1; len = 0; !partial, !*nfds => ret = 0; len < 0; partial, !*nfds => ret = -1; errmsg; len = 0; partial, !*nfds => ret = -1; errmsg; len < 0; partial, *nfds => ret = -1; errmsg, clearfds len < 0; !partial, *nfds => ret = -1; errmsg, clearfds len = 0; partial, *nfds => ret = -1; errmsg, clearfds len = 0; !partial, *nfds => ret = -1; errmsg, clearfds len = 0; !niov; (nfds && *nfds) => ret = 1 /* fds-only */ len > 0 => ret 1 Thank you! -- Jag
On Fri, Jan 15, 2021 at 01:19:01PM -0500, Jag Raman wrote: > > > > On Jan 15, 2021, at 4:20 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > > > On Thu, Jan 14, 2021 at 01:24:37PM -0500, Jag Raman wrote: > >> > >> > >>> On Jan 14, 2021, at 1:00 PM, Daniel P. Berrangé <berrange@redhat.com> wrote: > >>> > >>> On Thu, Jan 14, 2021 at 12:55:58PM -0500, Jag Raman wrote: > >>>> > >>>> > >>>>> On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > >>>>> > >>>>> On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote: > >>>>>> +int qio_channel_readv_full_all(QIOChannel *ioc, > >>>>>> + const struct iovec *iov, > >>>>>> + size_t niov, > >>>>>> + int **fds, size_t *nfds, > >>>>>> + Error **errp) > >>>>>> { > >>>>>> - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp); > >>>>>> + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp); > >>>>>> > >>>>>> if (ret == 0) { > >>>>>> - ret = -1; > >>>>>> error_setg(errp, > >>>>>> "Unexpected end-of-file before all bytes were read"); > >>>>> > >>>>> qio_channel_readv_full_all_eof() can read file descriptors but no data > >>>>> and return 0. > >>>>> > >>>>> Here that case is converted into an error and the file descriptors > >>>>> aren't closed, freed, and fds/nfds isn't cleared. > >>>> > >>>> That’s a valid point. I’m wondering if the fix for this case should be in > >>>> qio_channel_readv_full_all_eof(), instead of here. > >>>> > >>>> qio_channel_readv_full_all_eof() should probably return error (-1) if the > >>>> amount of data read does not match iov_size(). If the caller is only expecting > >>>> to read fds, and not any data, it would indicate that by setting iov to NULL > >>>> and/or setting niov=0. If the caller is setting these parameters, it means it is > >>>> expecting data.Does that sound good? > >>> > >>> The API spec for the existing _eof() methods says: > >>> > >>> * The function will wait for all requested data > >>> * to be read, yielding from the current coroutine > >>> * if required. > >>> * > >>> * If end-of-file occurs before any data is read, > >>> * no error is reported; otherwise, if it occurs > >>> * before all requested data has been read, an error > >>> * will be reported. > >>> > >>> > >>> IOW, return '0' is *only* valid if we've not read anything. I consider > >>> file descriptors to be something. > >>> > >>> IOW, qio_channel_readv_full_all_eof must only return 0, if it didn't > >>> read any data and also didn't receive any file descriptors. So yeah, > >>> we must return -1 in the scenario Stefan describes > >> > >> That makes sense to me. Reading “fds" is something, which is different > >> from our previous understanding. I thought data only meant iov, and not fds. > >> > >> So the return values for qio_channel_readv_full_all_eof() would be: > >> - ‘0’ only if EOF is reached without reading any fds and data. > >> - ‘1’ if all data that the caller expects are read (even if the caller reads > >> fds exclusively, without any iovs) > >> - ‘-1’ otherwise, considered as error > >> > >> qio_channel_readv_full_all() would return: > >> - ‘0’ if all the data that caller expects are read > >> - ‘-1’ otherwise, considered as error > >> > >> Hey Stefan, > >> > >> Does this sound good to you? > > > > The while (nlocal_iov > 0) loop only runs if the caller has requested to > > read at least some data, so the fds-only case doesn't work yet. > > > > This suggests that no current QEMU code relies on the fds-only case. > > Therefore you could change the doc comment to clarify this instead of > > adding support for this case to the code. > > > > But if you would to fully support the fds-only case that would be even > > better. > > > > Stefan > > We are working on sending the next revision out. We could handle the > fds-only case by altering the while loop condition to be: > ((nlocal_iov > 0) || local_fds) > > For reference, we would need to handle the following cases: > len < 0; !partial, !*nfds => ret = -1; > len = 0; !partial, !*nfds => ret = 0; > len < 0; partial, !*nfds => ret = -1; errmsg; > len = 0; partial, !*nfds => ret = -1; errmsg; > len < 0; partial, *nfds => ret = -1; errmsg, clearfds > len < 0; !partial, *nfds => ret = -1; errmsg, clearfds > len = 0; partial, *nfds => ret = -1; errmsg, clearfds > len = 0; !partial, *nfds => ret = -1; errmsg, clearfds > len = 0; !niov; (nfds && *nfds) => ret = 1 /* fds-only */ > len > 0 => ret 1 Yes, I think that looks right. Stefan
diff --git a/include/io/channel.h b/include/io/channel.h index 19e76fc..f725665 100644 --- a/include/io/channel.h +++ b/include/io/channel.h @@ -778,6 +778,57 @@ void qio_channel_set_aio_fd_handler(QIOChannel *ioc, void *opaque); /** + * qio_channel_readv_full_all_eof: + * @ioc: the channel object + * @iov: the array of memory regions to read data to + * @niov: the length of the @iov array + * @fds: an array of file handles to read + * @nfds: number of file handles in @fds + * @errp: pointer to a NULL-initialized error object + * + * + * Performs same function as qio_channel_readv_all_eof. + * Additionally, attempts to read file descriptors shared + * over the channel. The function will wait for all + * requested data to be read, yielding from the current + * coroutine if required. + * + * Returns: 1 if all bytes were read, 0 if end-of-file + * occurs without data, or -1 on error + */ + +int qio_channel_readv_full_all_eof(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int **fds, size_t *nfds, + Error **errp); + +/** + * qio_channel_readv_full_all: + * @ioc: the channel object + * @iov: the array of memory regions to read data to + * @niov: the length of the @iov array + * @fds: an array of file handles to read + * @nfds: number of file handles in @fds + * @errp: pointer to a NULL-initialized error object + * + * + * Performs same function as qio_channel_readv_all_eof. + * Additionally, attempts to read file descriptors shared + * over the channel. The function will wait for all + * requested data to be read, yielding from the current + * coroutine if required. + * + * Returns: 0 if all bytes were read, or -1 on error + */ + +int qio_channel_readv_full_all(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int **fds, size_t *nfds, + Error **errp); + +/** * qio_channel_writev_full_all: * @ioc: the channel object * @iov: the array of memory regions to write data from diff --git a/io/channel.c b/io/channel.c index 0d4b8b5..8d3341f 100644 --- a/io/channel.c +++ b/io/channel.c @@ -92,19 +92,47 @@ int qio_channel_readv_all_eof(QIOChannel *ioc, size_t niov, Error **errp) { + return qio_channel_readv_full_all_eof(ioc, iov, niov, NULL, NULL, errp); +} + +int qio_channel_readv_all(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + Error **errp) +{ + return qio_channel_readv_full_all(ioc, iov, niov, NULL, NULL, errp); +} + +int qio_channel_readv_full_all_eof(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int **fds, size_t *nfds, + Error **errp) +{ int ret = -1; struct iovec *local_iov = g_new(struct iovec, niov); struct iovec *local_iov_head = local_iov; unsigned int nlocal_iov = niov; + int **local_fds = fds; + size_t *local_nfds = nfds; bool partial = false; + if (nfds) { + *nfds = 0; + } + + if (fds) { + *fds = NULL; + } + nlocal_iov = iov_copy(local_iov, nlocal_iov, iov, niov, 0, iov_size(iov, niov)); while (nlocal_iov > 0) { ssize_t len; - len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp); + len = qio_channel_readv_full(ioc, local_iov, nlocal_iov, local_fds, + local_nfds, errp); if (len == QIO_CHANNEL_ERR_BLOCK) { if (qemu_in_coroutine()) { qio_channel_yield(ioc, G_IO_IN); @@ -112,20 +140,42 @@ int qio_channel_readv_all_eof(QIOChannel *ioc, qio_channel_wait(ioc, G_IO_IN); } continue; - } else if (len < 0) { - goto cleanup; - } else if (len == 0) { - if (partial) { - error_setg(errp, - "Unexpected end-of-file before all bytes were read"); - } else { + } + + if (len <= 0) { + size_t fd_idx = 0; + + if ((len == 0) && !partial) { ret = 0; + goto cleanup; + } else if (len == 0) { + error_prepend(errp, + "Unexpected end-of-file before all bytes were read."); } + + if (nfds) { + fd_idx = *nfds; + *nfds = 0; + } + + while (fds && fd_idx) { + close((*fds)[fd_idx - 1]); + fd_idx--; + } + + if (fds) { + g_free(*fds); + *fds = NULL; + } + goto cleanup; } partial = true; iov_discard_front(&local_iov, &nlocal_iov, len); + + local_fds = NULL; + local_nfds = 0; } ret = 1; @@ -135,20 +185,23 @@ int qio_channel_readv_all_eof(QIOChannel *ioc, return ret; } -int qio_channel_readv_all(QIOChannel *ioc, - const struct iovec *iov, - size_t niov, - Error **errp) +int qio_channel_readv_full_all(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int **fds, size_t *nfds, + Error **errp) { - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp); + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp); if (ret == 0) { - ret = -1; error_setg(errp, "Unexpected end-of-file before all bytes were read"); - } else if (ret == 1) { - ret = 0; + return -1; } + if (ret == 1) { + return 0; + } + return ret; }