| Message ID | 20200820211905.223523-1-nsoffer@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu-iotests: Fix FilePaths cleanup | expand |
On Fri, Aug 21, 2020 at 12:19 AM Nir Soffer <nirsof@gmail.com> wrote: > > If os.remove() fails to remove one of the paths, for example if the file > was removed by the test, the cleanup loop would exit silently, without > removing the rest of the files. > > Signed-off-by: Nir Soffer <nsoffer@redhat.com> > --- > dtc | 2 +- > tests/qemu-iotests/iotests.py | 8 ++++---- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/dtc b/dtc > index 85e5d83984..88f18909db 160000 > --- a/dtc > +++ b/dtc > @@ -1 +1 @@ > -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647 > +Subproject commit 88f18909db731a627456f26d779445f84e449536 This sneaked into the patch somehow, I did not change this. > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index 717b5b652c..16a04df8a3 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -468,11 +468,11 @@ class FilePaths: > return self.paths > > def __exit__(self, exc_type, exc_val, exc_tb): > - try: > - for path in self.paths: > + for path in self.paths: > + try: > os.remove(path) > - except OSError: > - pass > + except OSError: > + pass > return False > > class FilePath(FilePaths): > -- > 2.26.2 >
On 8/20/20 4:19 PM, Nir Soffer wrote: > If os.remove() fails to remove one of the paths, for example if the file > was removed by the test, the cleanup loop would exit silently, without > removing the rest of the files. > > Signed-off-by: Nir Soffer <nsoffer@redhat.com> > --- > dtc | 2 +- > tests/qemu-iotests/iotests.py | 8 ++++---- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/dtc b/dtc > index 85e5d83984..88f18909db 160000 > --- a/dtc > +++ b/dtc > @@ -1 +1 @@ > -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647 > +Subproject commit 88f18909db731a627456f26d779445f84e449536 Modulo the unintended submodule bump, > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index 717b5b652c..16a04df8a3 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -468,11 +468,11 @@ class FilePaths: > return self.paths > > def __exit__(self, exc_type, exc_val, exc_tb): > - try: > - for path in self.paths: > + for path in self.paths: > + try: > os.remove(path) > - except OSError: > - pass > + except OSError: > + pass > return False Reviewed-by: Eric Blake <eblake@redhat.com>
On 8/20/20 4:29 PM, Eric Blake wrote: > On 8/20/20 4:19 PM, Nir Soffer wrote: >> If os.remove() fails to remove one of the paths, for example if the file >> was removed by the test, the cleanup loop would exit silently, without >> removing the rest of the files. >> >> Signed-off-by: Nir Soffer <nsoffer@redhat.com> >> --- >> dtc | 2 +- >> tests/qemu-iotests/iotests.py | 8 ++++---- >> 2 files changed, 5 insertions(+), 5 deletions(-) > > Reviewed-by: Eric Blake <eblake@redhat.com> By the way, what test did you hit this on? If possible, I'd like to add a Fixes: tag pointing to a commit that includes the problem.
On Fri, Aug 21, 2020 at 12:33 AM Eric Blake <eblake@redhat.com> wrote: > > On 8/20/20 4:29 PM, Eric Blake wrote: > > On 8/20/20 4:19 PM, Nir Soffer wrote: > >> If os.remove() fails to remove one of the paths, for example if the file > >> was removed by the test, the cleanup loop would exit silently, without > >> removing the rest of the files. > >> > >> Signed-off-by: Nir Soffer <nsoffer@redhat.com> > >> --- > >> dtc | 2 +- > >> tests/qemu-iotests/iotests.py | 8 ++++---- > >> 2 files changed, 5 insertions(+), 5 deletions(-) > > > > > Reviewed-by: Eric Blake <eblake@redhat.com> > > By the way, what test did you hit this on? If possible, I'd like to add > a Fixes: tag pointing to a commit that includes the problem. I did not hit this issue, found it while reviewing another patch, while trying to understand what FilePath is doing. The error was introduced in: commit de263986b5dc7571d12a95305ffc7ddd2f349431 Author: John Snow <jsnow@redhat.com> Date: Mon Jul 29 16:35:54 2019 -0400 iotests: teach FilePath to produce multiple paths Use "FilePaths" instead of "FilePath" to request multiple files be cleaned up after we leave that object's scope. This is not crucial; but it saves a little typing. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20190709232550.10724-16-jsnow@redhat.com Signed-off-by: John Snow <jsnow@redhat.com>
On Fri, Aug 21, 2020 at 12:40 AM Nir Soffer <nsoffer@redhat.com> wrote: > > On Fri, Aug 21, 2020 at 12:33 AM Eric Blake <eblake@redhat.com> wrote: > > > > On 8/20/20 4:29 PM, Eric Blake wrote: > > > On 8/20/20 4:19 PM, Nir Soffer wrote: > > >> If os.remove() fails to remove one of the paths, for example if the file > > >> was removed by the test, the cleanup loop would exit silently, without > > >> removing the rest of the files. > > >> > > >> Signed-off-by: Nir Soffer <nsoffer@redhat.com> > > >> --- > > >> dtc | 2 +- > > >> tests/qemu-iotests/iotests.py | 8 ++++---- > > >> 2 files changed, 5 insertions(+), 5 deletions(-) > > > > > > > > Reviewed-by: Eric Blake <eblake@redhat.com> > > > > By the way, what test did you hit this on? If possible, I'd like to add > > a Fixes: tag pointing to a commit that includes the problem. I'll send a v2 with a Fixes tag, and few other related fixes. > > I did not hit this issue, found it while reviewing another patch, > while trying to > understand what FilePath is doing. > > The error was introduced in: > > commit de263986b5dc7571d12a95305ffc7ddd2f349431 > Author: John Snow <jsnow@redhat.com> > Date: Mon Jul 29 16:35:54 2019 -0400 > > iotests: teach FilePath to produce multiple paths > > Use "FilePaths" instead of "FilePath" to request multiple files be > cleaned up after we leave that object's scope. > > This is not crucial; but it saves a little typing. > > Signed-off-by: John Snow <jsnow@redhat.com> > Reviewed-by: Max Reitz <mreitz@redhat.com> > Message-id: 20190709232550.10724-16-jsnow@redhat.com > Signed-off-by: John Snow <jsnow@redhat.com>
diff --git a/dtc b/dtc index 85e5d83984..88f18909db 160000 --- a/dtc +++ b/dtc @@ -1 +1 @@ -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647 +Subproject commit 88f18909db731a627456f26d779445f84e449536 diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 717b5b652c..16a04df8a3 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -468,11 +468,11 @@ class FilePaths: return self.paths def __exit__(self, exc_type, exc_val, exc_tb): - try: - for path in self.paths: + for path in self.paths: + try: os.remove(path) - except OSError: - pass + except OSError: + pass return False class FilePath(FilePaths):
If os.remove() fails to remove one of the paths, for example if the file was removed by the test, the cleanup loop would exit silently, without removing the rest of the files. Signed-off-by: Nir Soffer <nsoffer@redhat.com> --- dtc | 2 +- tests/qemu-iotests/iotests.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)