Repository: moby/moby
File: volume/store/store.go (Go)

Code before:
// It is expected that callers of this function hold any necessary locks.
func (s *VolumeStore) create(name, driverName string, opts, labels map[string]string) (volume.Volume, error) {
	// Validate the name in a platform-specific manner
	valid, err := volume.IsVolumeNameValid(name)
	if err != nil {
		return nil, err
	}
	if !valid {
		return nil, &OpErr{Err: errInvalidName, Name: name, Op: "create"}
	}

	if v, exists := s.getNamed(name); exists {
		if v.DriverName() != driverName && driverName != "" && driverName != volume.DefaultDriverName {
			return nil, errNameConflict
		}
		return v, nil
	}

	// Since there isn't a specified driver name, let's see if any of the existing drivers have this volume name
	if driverName == "" {
		v, _ := s.getVolume(name)
		if v != nil {
			return v, nil
		}
	}

	vd, err := volumedrivers.CreateDriver(driverName)

	if err != nil {
		return nil, &OpErr{Op: "create", Name: name, Err: err}
	}

	logrus.Debugf("Registering new volume reference: driver %q, name %q", vd.Name(), name)

	if v, _ := vd.Get(name); v != nil {
		return v, nil
	}
	v, err := vd.Create(name, opts)
	if err != nil {
		return nil, err
	}
	s.globalLock.Lock()
	s.labels[name] = labels
	s.options[name] = opts
	s.globalLock.Unlock()

	if s.db != nil {
		metadata := &volumeMetadata{
			Name:    name,
			Labels:  labels,
			Options: opts,

Review comment (suggestion):
`driverName` should be non-empty for this function, else error out. This should be checked before entering the loop.


Code after:

// Create creates a volume with the given name and driver.
// This is just like CreateWithRef() except we don't store a reference while holding the lock.
func (s *VolumeStore) Create(name, driverName string, opts, labels map[string]string) (volume.Volume, error) {
	return s.CreateWithRef(name, driverName, "", opts, labels)
}

// checkConflict checks the local cache for name collisions with the passed in name,
// for existing volumes with the same name but in a different driver.
// This is used by `Create` as a best effort to prevent name collisions for volumes.
// If a matching volume is found that is not a conflict that is returned so the caller
// does not need to perform an additional lookup.
// When no matching volume is found, both returns will be nil
//
// Note: This does not probe all the drivers for name collisions because v1 plugins
// are very slow, particularly if the plugin is down, and cause other issues,
// particularly around locking the store.
// TODO(cpuguy83): With v2 plugins this shouldn't be a problem. Could also potentially
// use a connect timeout for this kind of check to ensure we aren't blocking for a
// long time.
func (s *VolumeStore) checkConflict(name, driverName string) (volume.Volume, error) {
	// check the local cache
	v, _ := s.getNamed(name)
	if v != nil {
		vDriverName := v.DriverName()
		if driverName != "" && vDriverName != driverName {
			// we have what looks like a conflict
			// let's see if there are existing refs to this volume, if so we don't need
			// to go any further since we can assume the volume is legit.
			if len(s.getRefs(name)) > 0 {
				return nil, errors.Wrapf(errNameConflict, "driver '%s' already has volume '%s'", vDriverName, name)
			}

			// looks like there is a conflict, but nothing is referencing it...
			// let's check if the found volume ref
			// is stale by checking with the driver if it still exists
			vd, err := volumedrivers.GetDriver(vDriverName)
			if err != nil {
				// play it safe and return the error
				// TODO(cpuguy83): maybe when when v2 plugins are ubiquitous, we should
				// just purge this from the cache
				return nil, errors.Wrapf(errNameConflict, "found reference to volume '%s' in driver '%s', but got an error while checking the driver: %v", name, vDriverName, err)
			}

			// now check if it still exists in the driver
			v2, err := vd.Get(name)
			err = errors.Cause(err)
			if err != nil {
				if _, ok := err.(net.Error); ok {
					// got some error related to the driver connectivity
					// play it safe and return the error