Hacking has bumped the version of flake8 that it's using to 5.0 in its
6.0.1 release. This turns up quite a few pep8 errors lurking in our
code. Fix them.
Needed-by: https://review.opendev.org/c/openstack/hacking/+/874516
Change-Id: I3b9c7f9f5de757f818ec358c992ffb0e5f3e310f
autopep8 is a code formating tool that makes python code pep8
compliant without changing everything. Unlike black it will
not radically change all code and the primary change to the
existing codebase is adding a new line after class level doc strings.
This change adds a new tox autopep8 env to manually run it on your
code before you submit a patch, it also adds autopep8 to pre-commit
so if you use pre-commit it will do it for you automatically.
This change runs autopep8 in diff mode with --exit-code in the pep8
tox env so it will fail if autopep8 would modify your code if run
in in-place mode. This allows use to gate on autopep8 not modifying
patches that are submited. This will ensure authorship of patches is
maintianed.
The intent of this change is to save the large amount of time we spend
on ensuring style guidlines are followed automatically to make it
simpler for both new and old contibutors to work on nova and save
time and effort for all involved.
Change-Id: Idd618d634cc70ae8d58fab32f322e75bfabefb9d
libguestfs >= v1.41.1 and commit 0ee02e0117527 changed the return type
of read_file from string to bytes.
0ee02e0117
As we don't check the version of libguestfs installed this change
handles both the original behaviour where a string is returned and the
newer behaviour by decoding any returned bytes to a string.
Closes-Bug: #1882421
Change-Id: I1c12b2903c1e5bf3a88394493456ad33233f3cd8
The fix Iac8496065c8b6212d7edac320659444ab341b513 removed the last user
of VFSLocalFS so this patch remove the class, the related tests and all
the privsep functions that become dead code after this cleanup.
Change-Id: Ia1eb1d93d1f9699a4027b7a07107109ab9a3a29a
This is a security concern, as mounting filesystems on the host has
had previous CVEs around executing code on the host. libguestfs is
much safer, and is the only way we should allow this.
Some caveats came up during the discussion of the bug and this change
which are documented in the release note.
Co-Authored-By: Matt Riedemann <mriedem.os@gmail.com>
Closes-Bug: #1552042
Change-Id: Iac8496065c8b6212d7edac320659444ab341b513
We shouldn't be looking up the architecture of the test node during
tests to ensure tests work across nodes with varying architectures.
Change-Id: I458b1db091e33c0b835e44b5a86de6c0a08f99a3
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Co-authored-by: Lee Yarwood <lyarwood@redhat.com>
Replace six.text_type with str.
A subsequent patch will replace other six.text_type.
Change-Id: I23bb9e539d08f5c6202909054c2dd49b6c7a7a0e
Implements: blueprint six-removal
Signed-off-by: Takashi Natsume <takanattie@gmail.com>
We neither need nor want these translated so remove them. A couple of
logs with unnecessary brackets or weird indentation were identified in
the process, and these are also modified. Note that exceptions must
still be translated. Refer to [1] for more details.
[1] https://docs.openstack.org/oslo.i18n/latest/user/guidelines.html
Change-Id: I4573a7c5f3a7b5716a15fbd15ad9336807843a03
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
_detect_nbd_devices uses the filter
builtin internally to filter valid devices.
In python 2, filter returns a list. In python 3,
filter returns an iterable or generator function.
This change eagerly converts the result of calling filter
to a list to preserve the python 2 behaviour under python 3.
Closes-Bug: #1840068
Change-Id: I25616c5761ea625a15d725777ae58175651558f8
The call in nova.virt.disk.mount.api.Mount.mnt_dev() to
nova.privsep.fs.mount() should include the `options` argument to
fulfill with the method signature.
The test test_do_mount_need_to_specify_fs_type has been modified to
check that the caller use the correct signature.
Closes-Bug: 1829506
Change-Id: Id14993db6ea33b2da14caa4b58671fc57c182706
Signed-off-by: Miguel Herranz <miguel@midokura.com>
This method takes a list of strings. It was never failing
because python-libguestfs takes the input and casts it to
a list, which turns "force_tcg" into:
['f', 'o', 'r', 'c', 'e', '_', 't', 'c', 'g']
Which is obviously not correct.
http://libguestfs.org/guestfs.3.html#guestfs_set_backend_settings
This fixes that issue and leaves a TODO about investigating
the usage of set_backend_setting for just setting the
single force_tcg value instead of overrwriting all backend
settings.
Credit to Clark Boylan for identifying the issue.
Change-Id: I36edfaf30fd0cc234f929fd6937d07b6a1b646ae
Partial-Bug: #1735823
This brings in a couple of new checks which must be addressed, many of
which involve a rather large amount of changes, so these are ignored for
now. A series of follow-up changes will resolved these.
'pycodestyle' is added as a dependency rather than it being pulled in
transitively. This is necessary since we're using it in tests.
Change-Id: I35c654bd39f343417e0a1124263ff31dcd0b05c9
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
I464bc2b88123a012cd12213beac4b572c3c20a56 introduced a second call to
``qemu-img`` that can easily be collapsed into one with the addition of
a new call within the disk_api.
Related-Bug: #1785827
Change-Id: Ibfd0527ed79f60282b542034d7cb97b424becba3
At present the Libvirt driver can preallocate file based disks using the
fallocate command and importantly the `-n` option. This option allocates
blocks on the filesystem past the initial EOF of a given file:
```
$ touch test.img ; fallocate -n -l $(( 1024 * 1024 )) test.img
$ ll -lah test.img
-rw-rw-r--. 1 stack stack 0 Apr 16 13:28 test.img
$ du -h test.img
1.0M test.img
```
This results in a miscalculation of the total disk overcommit for file
based (excluding ploop) disks as os.path.getsize is currently used to
determine the allocated size of these disks:
```
>>> import os
>>> os.path.getsize('test.img')
0
```
Using the above example the disk overcommit would be reported as 1.0M as
the disk appears empty yet will report a potential (virtual) size of 1.0M.
However as the required blocks have already been allocated on the
filesystem the host will report disk_available_least as missing an
additional 1.0M, essentially doubling the allocation for each disk.
To correct this the allocated size of file based (excluding ploop) disks
is reported using `disk_size` from the `qemu-img info` command. This
should ensure blocks allocated past the EOF of the file are taken into
account and correctly reported as allocated.
A future change should ultimately remove the use of the `-n` option with
fallocate, however as this would not help disks that have already been
allocated this has not been included in this change to simplify backports.
Change-Id: If642e51a4e186833349a8e30b04224a3687f5594
Closes-bug: #1764489
The same pattern as the rest of the changes. This means that privsep now
needs to let you pass flags to e2fsck, which I don't love and will remove
in a later patch.
Change-Id: I6c695c04ae586fec6adc354257638116277dda88
blueprint: hurrah-for-privsep
Nova allows deployers to configure the command line which is used to create
a filesystem of a given type. This is frankly a little bit weird, but its
also historical. Move this functionality to privsep, including doing a
dance at startup to load config flags into privsep in a hopefully secure
manner.
Honestly, all of this code should be deprecated, but that's above my pay
grade and would take time to do. Oh, and maybe deployers love it the way
it is.
Change-Id: Id8eeb21e10f98a448946f178c8c5a36e48c7cac6
blueprint: hurrah-for-privsep
A similar pattern to the others. Maybe we should write a more fully fledged
wrapper around blockdev at some point, but I think its not required yet.
Change-Id: Ie65d7c90abd0f8448500089fa48e831aef7b7abf
blueprint: hurrah-for-privsep
This patch introduces the first code in the privsep directory which
_does_not_ run with escalated premissions. This is a requirement because
the privsep code has a restricted python path when executing. This
pattern will be used for other methods which are only sometimes
escalated.
Change-Id: Ie09e40d6476dcabda2d599e96701d419e3e8bdf0
blueprint: hurrah-for-privsep
When guestfs.debug is enabled, we're handling callback events
from guestfs and logging them at debug level. When guestfs
is launched to inspect capabilities, that is currently done
in an eventlet thread pool. Because of the concurrent logging
along with the eventlet thread, we can hit an issue where eventlet
tries to switch threads and fails and then we hang the launch
call to guestfs, which hangs creating an instance.
This change simply avoids using a thread pool to launch guestfs
if guestfs.debug is True.
Change-Id: I0ffe93a031154b123c8beff96a695df5a280b935
Closes-Bug: #1737214
According to the guestfs docs:
http://libguestfs.org/guestfs.3.html#guestfs_set_backend_settings
This method returns a 0 on success and -1 on failure. We suspect
we might not be calling this correctly, so log a warning if it
returns a non-zero return code.
Change-Id: I14a5969667a6f6f4ae529aaf0f6b9475fdf25496
Related-Bug: #1735823
The same pattern as previous patches. Some of these unit tests
are starting to be a bit simpler as we finish the transition.
Change-Id: If0e1fe4c0466f2f88525dc575af2ef366d4bb59d
blueprint: hurrah-for-privsep
Add a new filesytem mounting helper in privsep, and then start
moving things across to it. This currently implements mount and
unmount. We get to cleanup some rmdir calls while we're at it
which is nice as well.
I've added an upgrade note mentioning that we no longer ignore
the value of stderr from mount calls, as requesed in code review.
Change-Id: Ib5e585fa4bfb99617cd3ca983674114d323a3cce
blueprint: hurrah-for-privsep
The same pattern as the others, but with an added security concern.
Co-Authored-By: Evgeny Antyshev <eantyshev@virtuozzo.com>
Closes-Bug: #1717533
Change-Id: I1ac3a0ea4756ec68884866435c3da69171bbeb13
blueprint: hurrah-for-privsep
Having utilities that are named after the context they use that no
longer exists is going to be super confusing later. Move these to
a better place.
Change-Id: Id203aa6c02c3b486f63151b3607e928990a6ca7b
blueprint: hurrah-for-privsep
I don't particularly care about this use case (although the localfs
code should perhaps go away), but it was a nice contained example
of a privsep user which wasn't just calling a command line.
This patch also starts to layout what an API to the privsep'd code
might look like. For now its modelled on python's os module, because
that's where all the operations we perform are coming from.
The rootwrap configuration is cleaned up as we remove users.
Co-Authored-By: Tony Breeds <tony@bakeyournoodle.com>
Change-Id: I911cc51a226d6af29d63a7a2c69253de870073e9
The i18n team has decided not to translate the logs because it
seems like it not very useful; operators prefer to have them in
English so that they can search for those strings on the internet.
Partially fix on nova/virt other paths will be fixed on next commits
Change-Id: Ie7821aa4a5147cdb0616741bd1a1b1fc22080440
When setting ownership of a file or directory, the guestfs driver
looks for the /etc/passwd and/or /etc/group files. In case they
are not found, the current driver lets the auges RuntimeError
through, which does not produce a very helpful error message.
Fixing that by handling the original exception and rasing a
Nova exception with more details in it.
Related-bug: #1646002
Change-Id: I2d15865c8be13b938e10e67c1b1b160f2a80f0c0
Commit 92ae0f1 ("libvirt - Add log if libguestfs can't read host
kernel") reworks the logic of handling access to Kernel for libguestfs.
In doing that, it erroneously raises an exception when libguestfs is
_able_ to access the Kernel.
Fix it by reporting exception only when libguestfs does _not_ have
read access to the Kernel.
This was first tried by Kevin Zhao here
Ic6802650cb8f93e0d02c51e9014eb85a7e71f6fe, but is abandoned for some
reason.
This also adds a little more direction on what to do to fix this error.
Related-Bug: #1646002
Change-Id: Id6b4108e4e4af7c98b3e1bd9de3a09ad057e7b92
String interpolation should be delayed to be handled
by the logging code, rather than being done
at the point of the logging call.
So add the following hacking rule for it.
- [N354] String interpolation should be delayed at logging calls.
See the oslo i18n guideline.
* http://docs.openstack.org/developer/oslo.i18n/guidelines.html
Change-Id: Ief6d3ee3539c0857098fffdb7acfeec3e0fed6eb
Closes-Bug: #1596829
Adapt "nova resize" code to support Virtuozzo ploop disks.
As far as ploop disks are in fact directories we add '-r' argument
to all utilities that deal with instance' disks such as cp, rsync and scp.
Thus we copy disks universally whether they are folders or files.
Also using "prl_disk_tool" instead of "qemu-img" is better for ploop images
because it resizes guest filesystem as well.
We can't resize disks from guest OS in containers,
because they are not allowed to write directly to block device.
ploop tool can resize partition table and internal filesystem,
but only for container's disks. Such disks must have only one partition
with ext filesystem.
prl_disk_tool can resize disks with internal filesystems
and doesn't require any special layout so it can resize disks
for virtual machines. So it's better to use this tool instead of ploop.
Also we make compute.filters more strict
We call "ploop" only with "restore-descriptor" argument
And we set disk size in megabytes for prl_disk_tool
Co-Authored-By: Dmitry Guryanov <dguryanov@parallels.com>
Depends-On: I04c4379459c2fc1fd4801ec2aad53d0f6053b6d6
Change-Id: I38dbf73beb01fe1939ddca63fbfedbec1dc3c826
Implements: blueprint virtuozzo-instance-resize-support
Host's kernel only allows a root user to have read/write permission in
ubuntu. If compute-service didn't have read permission then libguestfs
will launch image fail.
In libguestfs offical FAQ site had point out this issue, following is
the link
http://libguestfs.org/guestfs-faq.1.html#binaries
It had suggested users to change host's kernel permission. But this
action should be handled by other patch. Here only give a hint what's
going wrong.
Change-Id: I36c93610831e2935d46f7ee37f95619fe6dc1481
Related-Bug: 1413142
Closes-Bug: 1491216
The config options of the "nova.conf" section "guestfs"
got moved to the new central location "nova/conf/guestfs.py".
Change-Id: I9db186e7d665fa7ce05e806843592d739fdd9c7c
Implements: centralize-config-options-newton
There's a race condition when trying to perform file injection without
libguestfs, which causes a fallback to nbd device. Although the kpartx
command succeeds, it does so after the code has tested for success, so
Nova thinks it failed.
Retry a few times to avoid this.
Co-Authored-By: Paul Carlton <paul.carlton2@hp.com>
Change-Id: Ie5c186562475cd56c55520ad7123f47a0130b2a4
Closes-Bug: #1428639
Closes-Bug: #1484586