Re: [PATCH v2 18/21] block, blksnap: snapshot

[Sergei Shtepa <sergei.shtepa@xxxxxxxxx>]

On 1/1/23 12:05, Hillf Danton wrote:
> Subject:
> Re: [PATCH v2 18/21] block, blksnap: snapshot
> From:
> Hillf Danton <hdanton@xxxxxxxx>
> Date:
> 1/1/23, 12:05
> 
> To:
> Sergei Shtepa <sergei.shtepa@xxxxxxxxx>
> CC:
> axboe@xxxxxxxxx, corbet@xxxxxxx, linux-mm@xxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
> 
> 
> On 9 Dec 2022 15:23:28 +0100 Sergei Shtepa <sergei.shtepa@xxxxxxxxx>
>> +int snapshot_create(struct blk_snap_dev *dev_id_array, unsigned int count,
>> +		    uuid_t *id)
>> +{
>> +	struct snapshot *snapshot = NULL;
>> +	int ret;
>> +	unsigned int inx;
>> +
>> +	pr_info("Create snapshot for devices:\n");
>> +	for (inx = 0; inx < count; ++inx)
>> +		pr_info("\t%u:%u\n", dev_id_array[inx].mj,
>> +			dev_id_array[inx].mn);
>> +
>> +	ret = check_same_devices(dev_id_array, count);
>> +	if (ret)
>> +		return ret;
>> +
>> +	snapshot = snapshot_new(count);
>> +	if (IS_ERR(snapshot)) {
>> +		pr_err("Unable to create snapshot: failed to allocate snapshot structure\n");
>> +		return PTR_ERR(snapshot);
>> +	}
>> +
>> +	ret = -ENODEV;
>> +	for (inx = 0; inx < count; ++inx) {
>> +		dev_t dev_id =
>> +			MKDEV(dev_id_array[inx].mj, dev_id_array[inx].mn);
>> +		struct tracker *tracker;
>> +
>> +		tracker = tracker_create_or_get(dev_id);
>> +		if (IS_ERR(tracker)) {
>> +			pr_err("Unable to create snapshot\n");
>> +			pr_err("Failed to add device [%u:%u] to snapshot tracking\n",
>> +			       MAJOR(dev_id), MINOR(dev_id));
>> +			ret = PTR_ERR(tracker);
>> +			goto fail;
>> +		}
>> +
>> +		snapshot->tracker_array[inx] = tracker;
>> +		snapshot->count++;
>> +	}
>> +
>> +	down_write(&snapshots_lock);
>> +	list_add_tail(&snapshots, &snapshot->link);
>> +	up_write(&snapshots_lock);
> Given list_for_each_entry below, typo wrt &snapshots found in the fresh 2023?
> 

Thanks.
It seems I just swapped the parameters by mistake.
It should be better:
```
	down_write(&snapshots_lock);
	list_add_tail(&snapshot->link, &snapshots);
	up_write(&snapshots_lock);
```

>> +
>> +	uuid_copy(id, &snapshot->id);
>> +	pr_info("Snapshot %pUb was created\n", &snapshot->id);
>> +	return 0;
>> +fail:
>> +	pr_err("Snapshot cannot be created\n");
>> +
>> +	snapshot_put(snapshot);
>> +	return ret;
>> +}
>> +
>> +static struct snapshot *snapshot_get_by_id(uuid_t *id)
>> +{
>> +	struct snapshot *snapshot = NULL;
>> +	struct snapshot *s;
>> +
>> +	down_read(&snapshots_lock);
>> +	if (list_empty(&snapshots))
>> +		goto out;
>> +
>> +	list_for_each_entry(s, &snapshots, link) {
>> +		if (uuid_equal(&s->id, id)) {
>> +			snapshot = s;
>> +			snapshot_get(snapshot);
>> +			break;
>> +		}
>> +	}
>> +out:
>> +	up_read(&snapshots_lock);
>> +	return snapshot;
>> +}