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

Host management Azure driver #1

Merged
merged 8 commits into from Oct 8, 2014

Conversation

Projects
None yet
3 participants
@jeffmendoza

jeffmendoza commented Oct 7, 2014

Please take a look, thanks.

ruslangabitov and others added some commits Oct 2, 2014

Added Azure driver to hosts
Added Azure driver to hosts
Added GetIP command and fixed GetStatus command
- Added GetIP command
- fixed GetStatus command
Added azure-sdk dep & Fixed bug with missing VM
Added azure-sdk dependency to hack\vendor.sh
Fixed bug with missing VM in drivers\azure
Changed Azure SDK dependency
Changed Azure SDK dependency. Now it points to v1.0 release
@bfirsh

This comment has been minimized.

Show comment
Hide comment
@bfirsh

bfirsh Oct 8, 2014

Owner

Great stuff, thanks Jeff!

Owner

bfirsh commented Oct 8, 2014

Great stuff, thanks Jeff!

bfirsh added a commit that referenced this pull request Oct 8, 2014

@bfirsh bfirsh merged commit b01f4ea into bfirsh:host-management Oct 8, 2014

bfirsh pushed a commit that referenced this pull request Oct 22, 2014

Fred Lifton
Merge pull request #1 from yosifkit/adding_official-repo-guidelines
Fix formatting, change to markdown block for long

bfirsh pushed a commit that referenced this pull request Jul 31, 2015

Simplify and fix os.MkdirAll() usage
TL;DR: check for IsExist(err) after a failed MkdirAll() is both
redundant and wrong -- so two reasons to remove it.

Quoting MkdirAll documentation:

> MkdirAll creates a directory named path, along with any necessary
> parents, and returns nil, or else returns an error. If path
> is already a directory, MkdirAll does nothing and returns nil.

This means two things:

1. If a directory to be created already exists, no error is returned.

2. If the error returned is IsExist (EEXIST), it means there exists
a non-directory with the same name as MkdirAll need to use for
directory. Example: we want to MkdirAll("a/b"), but file "a"
(or "a/b") already exists, so MkdirAll fails.

The above is a theory, based on quoted documentation and my UNIX
knowledge.

3. In practice, though, current MkdirAll implementation [1] returns
ENOTDIR in most of cases described in #2, with the exception when
there is a race between MkdirAll and someone else creating the
last component of MkdirAll argument as a file. In this very case
MkdirAll() will indeed return EEXIST.

Because of #1, IsExist check after MkdirAll is not needed.

Because of #2 and #3, ignoring IsExist error is just plain wrong,
as directory we require is not created. It's cleaner to report
the error now.

Note this error is all over the tree, I guess due to copy-paste,
or trying to follow the same usage pattern as for Mkdir(),
or some not quite correct examples on the Internet.

[v2: a separate aufs commit is merged into this one]

[1] https://github.com/golang/go/blob/f9ed2f75/src/os/path.go

Signed-off-by: Kir Kolyshkin <kir@openvz.org>

bfirsh pushed a commit that referenced this pull request Feb 2, 2016

Fix btrfs recursive btrfs subvol delete
Really fixing 2 things:

1. Panic when any error is detected while walking the btrfs graph dir on
removal due to no error check.
2. Nested subvolumes weren't actually being removed due to passing in
the wrong path

On point 2, for a path detected as a nested subvolume, we were calling
`subvolDelete("/path/to/subvol", "subvol")`, where the last part of the
path was duplicated due to a logic error, and as such actually causing
point #1 since `subvolDelete` joins the two arguemtns, and
`/path/to/subvol/subvol` (the joined version) doesn't exist.

Also adds a test for nested subvol delete.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment