Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Place dockvols in datastore root; locate VMs by uuid instead of name #451

Merged
merged 1 commit into from Jun 11, 2016

Conversation

msterin
Copy link
Contributor

@msterin msterin commented Jun 9, 2016

Fixes #449

  • Place dockvols in datastore root instesd of ../dockvols (which can be deeply nested)
  • Also , if VM as renamed, the whole "find VM" mechanism was confused - so I fixed by by using search by UUID rather than scan for name.
    • It has a side effect of being faster (on 10 VMs query time dropped from 0.25 sec to 0.05 sec

Test: make all , CI, manual

Manual test:

Running from VM located as /vmfs/volumes/datastore226/eek/nested/Clone1-Ubuntu 14desktop :

ESX - no dockvols in nested folders, as it is now created in the right place:

root@promc-2n-dhcp35> pwd
/vmfs/volumes/datastore226/eek/nested
root@promc-2n-dhcp35> ls
Clone1-Ubuntu 14desktop 

VM - we now see volumes created in other (not nested) VMs and can created stuff in the right place:

root@msterin-ubuntu14:~# docker volume create --driver=vmdk --name=YYY
YYY
root@msterin-ubuntu14:~# docker volume ls | grep vmdk
vmdk                XXX
vmdk                YYY
vmdk                vo1
vmdk                vol
vmdk                volumemore

ESX: - they volumes are indeed in the right place:

root@promc-2n-dhcp35> ls -lt /vmfs/volumes/datastore226/dockvols/
total 71680
-rw-------    1 root     root           118 Jun  9 23:49 YYY-18e55694a9d09109.vmfd
-rw-------    1 root     root     104857600 Jun  9 23:49 YYY-flat.vmdk
-rw-------    1 root     root           552 Jun  9 23:49 YYY.vmdk
-rw-------    1 root     root           118 Jun  9 23:45 XXX-319e3cdaefe14e31.vmfd
-rw-------    1 root     root     104857600 Jun  9 23:45 XXX-flat.vmdk
-rw-------    1 root     root           552 Jun  9 23:45 XXX.vmdk
-rw-------    1 root     root           118 Jun  9 23:24 volumemore-d3d00a6f02881136.vmfd
-rw-------    1 root     root     104857600 Jun  9 23:24 volumemore-flat.vmdk
-rw-------    1 root     root           566 Jun  9 23:24 volumemore.vmdk
-rw-------    1 root     root           118 Jun  9 23:24 vo1-5dcdd2cc28e940f0.vmfd
-rw-------    1 root     root     104857600 Jun  9 23:24 vo1-flat.vmdk
-rw-------    1 root     root           552 Jun  9 23:24 vo1.vmdk
-rw-------    1 root     root           118 Jun  9 23:24 vol-6d812701b9f8e2bf.vmfd
-rw-------    1 root     root     104857600 Jun  9 23:24 vol-flat.vmdk
-rw-------    1 root     root           552 Jun  9 23:24 vol.vmdk

…by Name

Place dockvols to datastore root - Fixe #449.
Also , if VM as renamed, the whole "find VM" mechanism was confused - so I fixed by by using search by UUID rather than
scan for name. It has a side effect of being faster (on 10 VMs query time dropped from 0.25 sec to 0.05 sec)
@govint
Copy link
Contributor

govint commented Jun 10, 2016

Looks good.

@pdhamdhere
Copy link
Contributor

Nice! LGTM. Thanks for quick turnaround.

PR description says, "update: oops, looks like volume mount/unmount still does not work. Investigating"

Have you already fixed it? I can't see why it would fail.

@msterin
Copy link
Contributor Author

msterin commented Jun 10, 2016

... I can't see why it would fail.

Me neither - but it fails to mount the attached disk with "no such file" - looks like a glitch in
(bus, device) -> /dev/by-path location mapping; I do not see how my fix would impact it but it's 100% reproducible so I'll get it tomorrow. I did not see it before submitting the PR and I'd like to keep it alive for tomorrow rather than withdraw and resubmit.

@kerneltime
Copy link
Contributor

kerneltime commented Jun 10, 2016

There is no tool/api call to get paths for datastores? This is the kind of stuff we hate to see on the other side when we want to change paths.. else LGTM

msterin> Yes, it exists but it would require connection and RPC call for a trivial string operation

@govint
Copy link
Contributor

govint commented Jun 10, 2016

The mapping doesn't exist in some old Ubuntu images.

@msterin msterin force-pushed the dockvol-location.msterin branch 2 times, most recently from 94689e6 to 75041ad Compare June 10, 2016 23:46
@msterin msterin changed the title Place dockvols to datastore root. Place dockvols to datastore root. Also locate VM by uuid, not by Name Jun 10, 2016
@msterin msterin force-pushed the dockvol-location.msterin branch 2 times, most recently from 55707d6 to 91b8352 Compare June 11, 2016 00:20
@msterin msterin changed the title Place dockvols to datastore root. Also locate VM by uuid, not by Name Place dockvols to datastore root; locate VMs by uuid instead of name Jun 11, 2016
@msterin msterin changed the title Place dockvols to datastore root; locate VMs by uuid instead of name Place dockvols in datastore root; locate VMs by uuid instead of name Jun 11, 2016

def find_child(vm_name):
e = None
# Return VM managed object, reconnect if needed. Throws is fails twice.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is => if

@pdhamdhere
Copy link
Contributor

Great catch on FindVmByName. LGTM. Some minor nits above.

logging.exception("Failed to find VM uuid=%s (traceback below), " \
"retrying...", vm_uuid)
#
# Retry. It can throw in connect/search fails. But search can't return None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in->if

@msterin msterin merged commit 9b32648 into master Jun 11, 2016
@msterin
Copy link
Contributor Author

msterin commented Jun 11, 2016

fixed nits and merged

@msterin msterin deleted the dockvol-location.msterin branch June 14, 2016 02:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect dockvols location for VMs in nested folders in datastore
5 participants