New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix qubes.storage.get_disk_usage(self.volumes[...]) in QubesVM #1961

Closed
woju opened this Issue May 5, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@woju
Member

woju commented May 5, 2016

There are three issues:

  1. get_disk_usage is in qubes.storage.file
    1.1. @marmarek is not sure if this is right place, because it may be used in other places. I (@woju) am however quite sure that yes, this is the right place, but it should be documented.
  2. There is no self.volumes anymore
  3. Maybe those functions should be removed, because they are already marked as deprecated.
vm/qubesvm.py-    def get_root_img_sz(self):
vm/qubesvm.py-        '''Get the size of the :py:attr:`volumes['root']`.
vm/qubesvm.py-
vm/qubesvm.py-        This is a temporary wrapper for backwards compatibility. You should
vm/qubesvm.py-        call directly :py:attr:`volumes[name].size`
vm/qubesvm.py-        :returns: domain's virtual disk size [FIXME unit]
vm/qubesvm.py-        :rtype: FIXME
vm/qubesvm.py-
vm/qubesvm.py-        .. seealso:: :py:meth:`get_disk_utilization_root_img`
vm/qubesvm.py-        '''
vm/qubesvm.py-
vm/qubesvm.py-        warnings.warn(
vm/qubesvm.py-            "get_disk_root_img_sz is deprecated, use volumes['root'].size",
vm/qubesvm.py-            DeprecationWarning)
vm/qubesvm.py:        return qubes.storage.get_disk_usage(self.volumes['root'].size)
vm/qubesvm.py-

@woju woju added C: core task labels May 5, 2016

@woju woju added this to the Release 4.0 milestone May 5, 2016

@woju woju added the bug label May 5, 2016

@kalkin

This comment has been minimized.

Show comment
Hide comment
@kalkin

kalkin May 5, 2016

Member

My idea was that in future if qubes-manager or anything else want to to know the disk size/usage it should just iterate over vm.volumes and call the appropriate fields for size and usage.

I would do the following:

  • remove old functions
  • document that each own implementation of volume needs size and usage properties.
  • Maybe remove the all the QubesVM utilization and size methods?
Member

kalkin commented May 5, 2016

My idea was that in future if qubes-manager or anything else want to to know the disk size/usage it should just iterate over vm.volumes and call the appropriate fields for size and usage.

I would do the following:

  • remove old functions
  • document that each own implementation of volume needs size and usage properties.
  • Maybe remove the all the QubesVM utilization and size methods?
@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek May 5, 2016

Member
  • Maybe remove the all the QubesVM utilization and size methods?

I'd leave them there. Or simply one method - get_disk_usage - overall disk usage of this VM. This method should summarize all the VM own volumes (but not for example root volume for AppVM). Detailed information can be get from vm.storage.

As for "own" definition - I'd use volume type here - exclude read-only and snapshot volumes .

Member

marmarek commented May 5, 2016

  • Maybe remove the all the QubesVM utilization and size methods?

I'd leave them there. Or simply one method - get_disk_usage - overall disk usage of this VM. This method should summarize all the VM own volumes (but not for example root volume for AppVM). Detailed information can be get from vm.storage.

As for "own" definition - I'd use volume type here - exclude read-only and snapshot volumes .

@woju

This comment has been minimized.

Show comment
Hide comment
@woju

woju May 5, 2016

Member

Not sure about read-only volumes, but it probably should exclude root volume from AppVM, externally attached (from qvm-block) etc. The reason is that it will be used not only by qubes-manager, but also by qvm-ls.

Member

woju commented May 5, 2016

Not sure about read-only volumes, but it probably should exclude root volume from AppVM, externally attached (from qvm-block) etc. The reason is that it will be used not only by qubes-manager, but also by qvm-ls.

@kalkin

This comment has been minimized.

Show comment
Hide comment
@kalkin

kalkin Aug 7, 2017

Member

I think this can be closed, it's an very old issue.

Member

kalkin commented Aug 7, 2017

I think this can be closed, it's an very old issue.

@kalkin kalkin closed this Aug 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment