Skip to content

fix(initiator): volume expansion avoid target disconnect & sync dm size #252

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.

Sign up for GitHub

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

Merged
derekbit merged 1 commit into from
Jan 2, 2026
Merged

fix(initiator): volume expansion avoid target disconnect & sync dm size #252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/cmd/nvmecli/nvmecli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func start(c *cli.Context) error {
return err
}

if _, err := i.StartNvmeTCPInitiator(c.String("traddr"), c.String("trsvcid"), true); err != nil {
if _, err := i.StartNvmeTCPInitiator(c.String("traddr"), c.String("trsvcid"), true, true); err != nil {
return err
}

Expand Down
60 changes: 55 additions & 5 deletions pkg/initiator/initiator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ const (
MaxUblkId = 65535
DefaultUblkQueueDepth = 128
DefaultUblkNumberOfQueue = 1

// LinuxKernelSectorSize is the fixed sector size (512 bytes) used by the
// Linux kernel for all block layer and Device Mapper table calculations.
// As defined in the kernel source (include/linux/types.h), "Linux always
// considers sectors to be 512 bytes long independently of the devices
// real block size."
//
// Refs:
// - https://github.com/torvalds/linux/blob/master/include/linux/types.h#L130-L138
// - https://android.googlesource.com/platform/external/lvm2/+/refs/heads/main/tools/dmsetup.c#103
DmSectorSize = 512

@derekbit derekbit Dec 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

default hard-coded value?

@davidcheng0922 davidcheng0922 Dec 31, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Investigate it

@davidcheng0922 davidcheng0922 Dec 31, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After investigating, it seems in linux kernel and dmsetup, it should always be 512, no matter the sector size of physical device.

/**
 * The type used for indexing onto a disc or disc partition.
 *
 * Linux always considers sectors to be 512 bytes long independently
 * of the devices real block size.
 *
 * blkcnt_t is the type of the inode's block count.
 */
typedef u64 sector_t;

ref: https://github.com/torvalds/linux/blob/master/include/linux/types.h#L130-L138

/* FIXME Should be elsewhere */
#define SECTOR_SHIFT 9L

ref: https://android.googlesource.com/platform/external/lvm2/+/refs/heads/main/tools/dmsetup.c#103

Do I still implement a helper function for this?

@davidcheng0922 davidcheng0922 Dec 31, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I'm wrong, please correct me, thank you

)

const (
Expand Down Expand Up @@ -292,7 +303,7 @@ func (i *Initiator) replaceDmDeviceTarget() error {
}

// StartNvmeTCPInitiator starts the NVMe-oF initiator with the given transportAddress and transportServiceID
func (i *Initiator) StartNvmeTCPInitiator(transportAddress, transportServiceID string, dmDeviceAndEndpointCleanupRequired bool) (dmDeviceIsBusy bool, err error) {
func (i *Initiator) StartNvmeTCPInitiator(transportAddress, transportServiceID string, dmDeviceAndEndpointCleanupRequired bool, stop bool) (dmDeviceIsBusy bool, err error) {
defer func() {
if err != nil {
err = errors.Wrapf(err, "failed to start NVMe-oF initiator %s", i.Name)
Expand Down Expand Up @@ -327,6 +338,12 @@ func (i *Initiator) StartNvmeTCPInitiator(transportAddress, transportServiceID s
err = i.LoadEndpointForNvmeTcpFrontend(false)
if err == nil {
i.logger.Info("NVMe-oF initiator is already launched with correct params")

// also load device source
err = i.waitAndLoadNVMeDeviceInfoWithoutLock(transportAddress, transportServiceID)
if err != nil {
i.logger.WithError(err).Warnf("Failed to load device info for NVMe-oF initiator %s", i.Name)
}
return false, nil
}
i.logger.WithError(err).Warnf("NVMe-oF initiator is launched with failed to load the endpoint")
Expand All @@ -335,10 +352,14 @@ func (i *Initiator) StartNvmeTCPInitiator(transportAddress, transportServiceID s
}
}

i.logger.Info("Stopping NVMe-oF initiator blindly before starting")
dmDeviceIsBusy, err = i.stopWithoutLock(nil, dmDeviceAndEndpointCleanupRequired, false, false)
if err != nil {
return dmDeviceIsBusy, errors.Wrapf(err, "failed to stop the mismatching NVMe-oF initiator %s before starting", i.Name)
if stop {
i.logger.Info("Stopping NVMe-oF initiator blindly before starting")
dmDeviceIsBusy, err = i.stopWithoutLock(nil, dmDeviceAndEndpointCleanupRequired, false, false)
if err != nil {
return dmDeviceIsBusy, errors.Wrapf(err, "failed to stop the mismatching NVMe-oF initiator %s before starting", i.Name)
}
} else {
dmDeviceIsBusy = true
}

i.logger.Info("Launching NVMe-oF initiator")
Expand Down Expand Up @@ -876,6 +897,35 @@ func (i *Initiator) ReloadDmDevice() (err error) {
return i.reloadLinearDmDevice()
}

func (i *Initiator) SyncDmDeviceSize(expectedSize uint64) error {
if i.dev == nil || i.dev.Source.Name == "" {
return fmt.Errorf("initiator device source is not initialized")
}

devPath := fmt.Sprintf("/dev/%s", i.dev.Source.Name)
expectedSectors := int64(expectedSize / DmSectorSize)

i.logger.Infof("Start reloading DM device %v to expected size %v bytes (%v sectors)", i.Name, expectedSize, expectedSectors)

var sectors int64
for retry := 0; retry < maxWaitDeviceRetries; retry++ {
output, err := i.executor.Execute(nil, util.BlockdevBinary, []string{"--getsize", devPath}, types.ExecuteTimeout)
if err == nil {
sectors, _ = strconv.ParseInt(strings.TrimSpace(output), 10, 64)
if sectors >= expectedSectors {
i.logger.Infof("Kernel updated device %v capacity to %v sectors", devPath, sectors)
break
}
}
time.Sleep(waitDeviceInterval)
}

if sectors < expectedSectors {
return fmt.Errorf("timeout waiting for device %v to reach expected size %v", devPath, expectedSectors)
}
return i.reloadLinearDmDevice()
}

// IsSuspended checks if the linear dm device is suspended
func (i *Initiator) IsSuspended() (bool, error) {
devices, err := util.DmsetupInfo(i.Name, i.executor)
Expand Down
4 changes: 2 additions & 2 deletions pkg/spdk/spdk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func (s *TestSuite) TestSPDKBasic(c *C) {
i, err := initiator.NewInitiator(raidName, initiator.HostProc, nvmeTCPInfo, nil)
c.Assert(err, IsNil)

dmDeviceBusy, err := i.StartNvmeTCPInitiator(types.LocalIP, defaultPort1, true)
dmDeviceBusy, err := i.StartNvmeTCPInitiator(types.LocalIP, defaultPort1, true, true)
c.Assert(dmDeviceBusy, Equals, false)
c.Assert(err, IsNil)
defer func() {
Expand Down Expand Up @@ -579,7 +579,7 @@ func (s *TestSuite) TestSPDKEngineSuspend(c *C) {
i, err := initiator.NewInitiator(raidName, initiator.HostProc, nvmeTCPInfo, nil)
c.Assert(err, IsNil)

dmDeviceBusy, err := i.StartNvmeTCPInitiator(types.LocalIP, defaultPort1, true)
dmDeviceBusy, err := i.StartNvmeTCPInitiator(types.LocalIP, defaultPort1, true, true)
c.Assert(dmDeviceBusy, Equals, false)
c.Assert(err, IsNil)
defer func() {
Expand Down
Loading