This fixes a Coverity report and improves the fid reclaim logic.
-----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEtIKLr5QxQM7yo0kQcdTV5YIvc9YFAmAOkVgACgkQcdTV5YIv c9YvThAAkyoz4rjSqPJrQTKV94DF3CIM2UnXm7CumvPOAb62JspCMY/EERv9bH+Q LCcBL+Evm1uimZwu3GWi5YIMNSKVaseIGhdH1bOurvqIsc27+QVuDA6C1eXl/eP5 NAjrLxrcAITo6dT8SzW4Pp6511RMzj00lI7IDlfCdWwn0rVF3DVGXowTrZyNyA9k Wlbs6LIXeI+/TCmVaB31HUnoh53S40NSrSb9OLGQUHFhbCEKMn/auGzsC0knZECo Ni3hDcnn3rD60s/LOHez8LkogmQ3lyvbjW68iEu3TQsMPFi8o8sFjocsI+7Uht7O enhXFtjtz8KJcYifQAtLUTUIUT4o02TDbmAxZSkEsi8bfuIwQtwtlYLt+r23RF4/ uT2ehYyJcyC51Q8undXa45396BFyvl5jRGfaHc5HTeqHEQgypbX2E+edJT6wTRLa uoP9t8VLpyDMLw2BPaGnLhbwDFuQ9qr/PSazRbpN5BX19pjhxmS0mRpJf7soSb4d QJmLGH8ZjW/BTMLmpgbZYY3k3khir+qH7WpzpGlCZdETAdaztpT9CaYb+BcZmSAf eYce9H3bkW9vlSsTUS9Tlb+kDsJICyOeGb0TVWKZ6gmDvypEiRL6wHsi1P3bT4kf cCzQBw4rERJx5pvjKGS4vn6Swz2rGBr17G/42A7Ng5aMaVkTqj8= =UPsj -----END PGP SIGNATURE----- Merge remote-tracking branch 'remotes/gkurz-gitlab/tags/9p-next-pull-request' into staging This fixes a Coverity report and improves the fid reclaim logic. # gpg: Signature made Mon 25 Jan 2021 09:37:28 GMT # gpg: using RSA key B4828BAF943140CEF2A3491071D4D5E5822F73D6 # gpg: Good signature from "Greg Kurz <groug@kaod.org>" [full] # gpg: aka "Gregory Kurz <gregory.kurz@free.fr>" [full] # gpg: aka "[jpeg image of size 3330]" [full] # Primary key fingerprint: B482 8BAF 9431 40CE F2A3 4910 71D4 D5E5 822F 73D6 * remotes/gkurz-gitlab/tags/9p-next-pull-request: 9pfs: Convert reclaim list to QSLIST 9pfs: Improve unreclaim loop 9pfs: Convert V9fsFidState::fid_list to QSIMPLEQ 9pfs: Convert V9fsFidState::clunked to bool 9pfs/proxy: Check return value of proxy_marshal() Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
commit
3dcfd4e3f2
|
@ -537,7 +537,8 @@ static int v9fs_request(V9fsProxy *proxy, int type, void *response, ...)
|
||||||
}
|
}
|
||||||
|
|
||||||
/* marshal the header details */
|
/* marshal the header details */
|
||||||
proxy_marshal(iovec, 0, "dd", header.type, header.size);
|
retval = proxy_marshal(iovec, 0, "dd", header.type, header.size);
|
||||||
|
assert(retval == 4 * 2);
|
||||||
header.size += PROXY_HDR_SZ;
|
header.size += PROXY_HDR_SZ;
|
||||||
|
|
||||||
retval = qemu_write_full(proxy->sockfd, iovec->iov_base, header.size);
|
retval = qemu_write_full(proxy->sockfd, iovec->iov_base, header.size);
|
||||||
|
|
102
hw/9pfs/9p.c
102
hw/9pfs/9p.c
|
@ -260,7 +260,7 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, int32_t fid)
|
||||||
V9fsFidState *f;
|
V9fsFidState *f;
|
||||||
V9fsState *s = pdu->s;
|
V9fsState *s = pdu->s;
|
||||||
|
|
||||||
for (f = s->fid_list; f; f = f->next) {
|
QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
|
||||||
BUG_ON(f->clunked);
|
BUG_ON(f->clunked);
|
||||||
if (f->fid == fid) {
|
if (f->fid == fid) {
|
||||||
/*
|
/*
|
||||||
|
@ -295,7 +295,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
|
||||||
{
|
{
|
||||||
V9fsFidState *f;
|
V9fsFidState *f;
|
||||||
|
|
||||||
for (f = s->fid_list; f; f = f->next) {
|
QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
|
||||||
/* If fid is already there return NULL */
|
/* If fid is already there return NULL */
|
||||||
BUG_ON(f->clunked);
|
BUG_ON(f->clunked);
|
||||||
if (f->fid == fid) {
|
if (f->fid == fid) {
|
||||||
|
@ -311,8 +311,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
|
||||||
* reclaim won't close the file descriptor
|
* reclaim won't close the file descriptor
|
||||||
*/
|
*/
|
||||||
f->flags |= FID_REFERENCED;
|
f->flags |= FID_REFERENCED;
|
||||||
f->next = s->fid_list;
|
QSIMPLEQ_INSERT_TAIL(&s->fid_list, f, next);
|
||||||
s->fid_list = f;
|
|
||||||
|
|
||||||
v9fs_readdir_init(s->proto_version, &f->fs.dir);
|
v9fs_readdir_init(s->proto_version, &f->fs.dir);
|
||||||
v9fs_readdir_init(s->proto_version, &f->fs_reclaim.dir);
|
v9fs_readdir_init(s->proto_version, &f->fs_reclaim.dir);
|
||||||
|
@ -401,29 +400,27 @@ static int coroutine_fn put_fid(V9fsPDU *pdu, V9fsFidState *fidp)
|
||||||
|
|
||||||
static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
|
static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
|
||||||
{
|
{
|
||||||
V9fsFidState **fidpp, *fidp;
|
V9fsFidState *fidp;
|
||||||
|
|
||||||
for (fidpp = &s->fid_list; *fidpp; fidpp = &(*fidpp)->next) {
|
QSIMPLEQ_FOREACH(fidp, &s->fid_list, next) {
|
||||||
if ((*fidpp)->fid == fid) {
|
if (fidp->fid == fid) {
|
||||||
break;
|
QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
|
||||||
}
|
fidp->clunked = true;
|
||||||
}
|
|
||||||
if (*fidpp == NULL) {
|
|
||||||
return NULL;
|
|
||||||
}
|
|
||||||
fidp = *fidpp;
|
|
||||||
*fidpp = fidp->next;
|
|
||||||
fidp->clunked = 1;
|
|
||||||
return fidp;
|
return fidp;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
|
|
||||||
void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
|
void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
|
||||||
{
|
{
|
||||||
int reclaim_count = 0;
|
int reclaim_count = 0;
|
||||||
V9fsState *s = pdu->s;
|
V9fsState *s = pdu->s;
|
||||||
V9fsFidState *f, *reclaim_list = NULL;
|
V9fsFidState *f;
|
||||||
|
QSLIST_HEAD(, V9fsFidState) reclaim_list =
|
||||||
|
QSLIST_HEAD_INITIALIZER(reclaim_list);
|
||||||
|
|
||||||
for (f = s->fid_list; f; f = f->next) {
|
QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
|
||||||
/*
|
/*
|
||||||
* Unlink fids cannot be reclaimed. Check
|
* Unlink fids cannot be reclaimed. Check
|
||||||
* for them and skip them. Also skip fids
|
* for them and skip them. Also skip fids
|
||||||
|
@ -453,8 +450,7 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
|
||||||
* a clunk request won't free this fid
|
* a clunk request won't free this fid
|
||||||
*/
|
*/
|
||||||
f->ref++;
|
f->ref++;
|
||||||
f->rclm_lst = reclaim_list;
|
QSLIST_INSERT_HEAD(&reclaim_list, f, reclaim_next);
|
||||||
reclaim_list = f;
|
|
||||||
f->fs_reclaim.fd = f->fs.fd;
|
f->fs_reclaim.fd = f->fs.fd;
|
||||||
f->fs.fd = -1;
|
f->fs.fd = -1;
|
||||||
reclaim_count++;
|
reclaim_count++;
|
||||||
|
@ -466,8 +462,7 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
|
||||||
* a clunk request won't free this fid
|
* a clunk request won't free this fid
|
||||||
*/
|
*/
|
||||||
f->ref++;
|
f->ref++;
|
||||||
f->rclm_lst = reclaim_list;
|
QSLIST_INSERT_HEAD(&reclaim_list, f, reclaim_next);
|
||||||
reclaim_list = f;
|
|
||||||
f->fs_reclaim.dir.stream = f->fs.dir.stream;
|
f->fs_reclaim.dir.stream = f->fs.dir.stream;
|
||||||
f->fs.dir.stream = NULL;
|
f->fs.dir.stream = NULL;
|
||||||
reclaim_count++;
|
reclaim_count++;
|
||||||
|
@ -481,15 +476,14 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
|
||||||
* Now close the fid in reclaim list. Free them if they
|
* Now close the fid in reclaim list. Free them if they
|
||||||
* are already clunked.
|
* are already clunked.
|
||||||
*/
|
*/
|
||||||
while (reclaim_list) {
|
while (!QSLIST_EMPTY(&reclaim_list)) {
|
||||||
f = reclaim_list;
|
f = QSLIST_FIRST(&reclaim_list);
|
||||||
reclaim_list = f->rclm_lst;
|
QSLIST_REMOVE(&reclaim_list, f, V9fsFidState, reclaim_next);
|
||||||
if (f->fid_type == P9_FID_FILE) {
|
if (f->fid_type == P9_FID_FILE) {
|
||||||
v9fs_co_close(pdu, &f->fs_reclaim);
|
v9fs_co_close(pdu, &f->fs_reclaim);
|
||||||
} else if (f->fid_type == P9_FID_DIR) {
|
} else if (f->fid_type == P9_FID_DIR) {
|
||||||
v9fs_co_closedir(pdu, &f->fs_reclaim);
|
v9fs_co_closedir(pdu, &f->fs_reclaim);
|
||||||
}
|
}
|
||||||
f->rclm_lst = NULL;
|
|
||||||
/*
|
/*
|
||||||
* Now drop the fid reference, free it
|
* Now drop the fid reference, free it
|
||||||
* if clunked.
|
* if clunked.
|
||||||
|
@ -502,32 +496,50 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path)
|
||||||
{
|
{
|
||||||
int err;
|
int err;
|
||||||
V9fsState *s = pdu->s;
|
V9fsState *s = pdu->s;
|
||||||
V9fsFidState *fidp;
|
V9fsFidState *fidp, *fidp_next;
|
||||||
|
|
||||||
again:
|
fidp = QSIMPLEQ_FIRST(&s->fid_list);
|
||||||
for (fidp = s->fid_list; fidp; fidp = fidp->next) {
|
if (!fidp) {
|
||||||
if (fidp->path.size != path->size) {
|
return 0;
|
||||||
continue;
|
|
||||||
}
|
}
|
||||||
if (!memcmp(fidp->path.data, path->data, path->size)) {
|
|
||||||
|
/*
|
||||||
|
* v9fs_reopen_fid() can yield : a reference on the fid must be held
|
||||||
|
* to ensure its pointer remains valid and we can safely pass it to
|
||||||
|
* QSIMPLEQ_NEXT(). The corresponding put_fid() can also yield so
|
||||||
|
* we must keep a reference on the next fid as well. So the logic here
|
||||||
|
* is to get a reference on a fid and only put it back during the next
|
||||||
|
* iteration after we could get a reference on the next fid. Start with
|
||||||
|
* the first one.
|
||||||
|
*/
|
||||||
|
for (fidp->ref++; fidp; fidp = fidp_next) {
|
||||||
|
if (fidp->path.size == path->size &&
|
||||||
|
!memcmp(fidp->path.data, path->data, path->size)) {
|
||||||
/* Mark the fid non reclaimable. */
|
/* Mark the fid non reclaimable. */
|
||||||
fidp->flags |= FID_NON_RECLAIMABLE;
|
fidp->flags |= FID_NON_RECLAIMABLE;
|
||||||
|
|
||||||
/* reopen the file/dir if already closed */
|
/* reopen the file/dir if already closed */
|
||||||
err = v9fs_reopen_fid(pdu, fidp);
|
err = v9fs_reopen_fid(pdu, fidp);
|
||||||
if (err < 0) {
|
if (err < 0) {
|
||||||
|
put_fid(pdu, fidp);
|
||||||
return err;
|
return err;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fidp_next = QSIMPLEQ_NEXT(fidp, next);
|
||||||
|
|
||||||
|
if (fidp_next) {
|
||||||
/*
|
/*
|
||||||
* Go back to head of fid list because
|
* Ensure the next fid survives a potential clunk request during
|
||||||
* the list could have got updated when
|
* put_fid() below and v9fs_reopen_fid() in the next iteration.
|
||||||
* switched to the worker thread
|
|
||||||
*/
|
*/
|
||||||
if (err == 0) {
|
fidp_next->ref++;
|
||||||
goto again;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* We're done with this fid */
|
||||||
|
put_fid(pdu, fidp);
|
||||||
}
|
}
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -537,14 +549,14 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
|
||||||
V9fsFidState *fidp;
|
V9fsFidState *fidp;
|
||||||
|
|
||||||
/* Free all fids */
|
/* Free all fids */
|
||||||
while (s->fid_list) {
|
while (!QSIMPLEQ_EMPTY(&s->fid_list)) {
|
||||||
/* Get fid */
|
/* Get fid */
|
||||||
fidp = s->fid_list;
|
fidp = QSIMPLEQ_FIRST(&s->fid_list);
|
||||||
fidp->ref++;
|
fidp->ref++;
|
||||||
|
|
||||||
/* Clunk fid */
|
/* Clunk fid */
|
||||||
s->fid_list = fidp->next;
|
QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
|
||||||
fidp->clunked = 1;
|
fidp->clunked = true;
|
||||||
|
|
||||||
put_fid(pdu, fidp);
|
put_fid(pdu, fidp);
|
||||||
}
|
}
|
||||||
|
@ -3121,7 +3133,7 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp,
|
||||||
* Fixup fid's pointing to the old name to
|
* Fixup fid's pointing to the old name to
|
||||||
* start pointing to the new name
|
* start pointing to the new name
|
||||||
*/
|
*/
|
||||||
for (tfidp = s->fid_list; tfidp; tfidp = tfidp->next) {
|
QSIMPLEQ_FOREACH(tfidp, &s->fid_list, next) {
|
||||||
if (v9fs_path_is_ancestor(&fidp->path, &tfidp->path)) {
|
if (v9fs_path_is_ancestor(&fidp->path, &tfidp->path)) {
|
||||||
/* replace the name */
|
/* replace the name */
|
||||||
v9fs_fix_path(&tfidp->path, &new_path, strlen(fidp->path.data));
|
v9fs_fix_path(&tfidp->path, &new_path, strlen(fidp->path.data));
|
||||||
|
@ -3215,7 +3227,7 @@ static int coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir,
|
||||||
* Fixup fid's pointing to the old name to
|
* Fixup fid's pointing to the old name to
|
||||||
* start pointing to the new name
|
* start pointing to the new name
|
||||||
*/
|
*/
|
||||||
for (tfidp = s->fid_list; tfidp; tfidp = tfidp->next) {
|
QSIMPLEQ_FOREACH(tfidp, &s->fid_list, next) {
|
||||||
if (v9fs_path_is_ancestor(&oldpath, &tfidp->path)) {
|
if (v9fs_path_is_ancestor(&oldpath, &tfidp->path)) {
|
||||||
/* replace the name */
|
/* replace the name */
|
||||||
v9fs_fix_path(&tfidp->path, &newpath, strlen(oldpath.data));
|
v9fs_fix_path(&tfidp->path, &newpath, strlen(oldpath.data));
|
||||||
|
@ -4081,7 +4093,7 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
|
||||||
s->ctx.fmode = fse->fmode;
|
s->ctx.fmode = fse->fmode;
|
||||||
s->ctx.dmode = fse->dmode;
|
s->ctx.dmode = fse->dmode;
|
||||||
|
|
||||||
s->fid_list = NULL;
|
QSIMPLEQ_INIT(&s->fid_list);
|
||||||
qemu_co_rwlock_init(&s->rename_lock);
|
qemu_co_rwlock_init(&s->rename_lock);
|
||||||
|
|
||||||
if (s->ops->init(&s->ctx, errp) < 0) {
|
if (s->ops->init(&s->ctx, errp) < 0) {
|
||||||
|
|
|
@ -279,9 +279,9 @@ struct V9fsFidState {
|
||||||
int open_flags;
|
int open_flags;
|
||||||
uid_t uid;
|
uid_t uid;
|
||||||
int ref;
|
int ref;
|
||||||
int clunked;
|
bool clunked;
|
||||||
V9fsFidState *next;
|
QSIMPLEQ_ENTRY(V9fsFidState) next;
|
||||||
V9fsFidState *rclm_lst;
|
QSLIST_ENTRY(V9fsFidState) reclaim_next;
|
||||||
};
|
};
|
||||||
|
|
||||||
typedef enum AffixType_t {
|
typedef enum AffixType_t {
|
||||||
|
@ -339,7 +339,7 @@ typedef struct {
|
||||||
struct V9fsState {
|
struct V9fsState {
|
||||||
QLIST_HEAD(, V9fsPDU) free_list;
|
QLIST_HEAD(, V9fsPDU) free_list;
|
||||||
QLIST_HEAD(, V9fsPDU) active_list;
|
QLIST_HEAD(, V9fsPDU) active_list;
|
||||||
V9fsFidState *fid_list;
|
QSIMPLEQ_HEAD(, V9fsFidState) fid_list;
|
||||||
FileOperations *ops;
|
FileOperations *ops;
|
||||||
FsContext ctx;
|
FsContext ctx;
|
||||||
char *tag;
|
char *tag;
|
||||||
|
|
Loading…
Reference in a new issue